Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

[16.0][OU-ADD] stock_account: set display_type of cogs account move line #4293

Merged
merged 3 commits into from
Mar 7, 2024

Conversation

robinkeunen
Copy link
Contributor

@robinkeunen robinkeunen commented Feb 2, 2024

superseeds #3971

Nothing to do in stock_account regarding the models and views.

Fill the display type for journal items corresponding to Cost of Good Sold lines (COGS) for customer invoices. In v15, the lines were set to is_anglo_saxon_line == True, in v16 field display_display is used.

openupgrade account migration script already sets display_type in _account_move_fast_fill_display_type (pre-migration)

I struggled setting up a database issuing cogs lines so I relied on my understanding of the code to write the sql command.

@robinkeunen robinkeunen force-pushed the v16_mig_stock_account branch 2 times, most recently from a8184ff to 9621088 Compare February 2, 2024 16:26
@robinkeunen robinkeunen changed the title [16.0][OU-ADD] stock_account: nothing to do #3971 [16.0][OU-ADD] stock_account: set display_type of cogs account move line #3971 Feb 2, 2024
docsource/modules150-160.rst Outdated Show resolved Hide resolved
@robinkeunen robinkeunen changed the title [16.0][OU-ADD] stock_account: set display_type of cogs account move line #3971 [16.0][OU-ADD] stock_account: set display_type of cogs account move line Feb 8, 2024
@robinkeunen robinkeunen force-pushed the v16_mig_stock_account branch from 9621088 to 6d00458 Compare February 8, 2024 17:43
Copy link
Contributor

@remytms remytms left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM. Tests does not pass, but it seams related to the stock module.

@MiquelRForgeFlow MiquelRForgeFlow added this to the 16.0 milestone Feb 15, 2024
Copy link

@andreampiovesana andreampiovesana left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM

@pedrobaeza
Copy link
Member

/ocabot migration stock_account

stock_account / account.move.line / stock_valuation_layer_ids (one2many): NEW relation: stock.valuation.layer
stock_account / stock.valuation.layer / account_move_line_id (many2one): NEW relation: account.move.line
stock_account / stock.valuation.layer / price_diff_value (float) : NEW
# NOTHING TO DO: new feature
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Shouldn't it be pointed that this would be addresed in purchase_stock (I'm not sure if it's necessary/possible/worth it) to rebuild past values

hoangtiendung070797 and others added 3 commits March 1, 2024 16:07
In v15, the lines were set to is_anglo_saxon_line == True
in v16, field display_display is used.
@OCA-git-bot
Copy link
Contributor

This PR has the approved label and has been created more than 5 days ago. It should therefore be ready to merge by a maintainer (or a PSC member if the concerned addon has no declared maintainer). 🤖

@pedrobaeza
Copy link
Member

The explanations on the work file are very few, but let's move on with this. price_diff_value should be reviewed later.

@pedrobaeza pedrobaeza merged commit 84b45b2 into OCA:16.0 Mar 7, 2024
2 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

9 participants