Skip to content

[18.0][MIG] stock_inventory_valuation_report#472

Open
oscars8a wants to merge 19 commits into
OCA:18.0from
oscars8a:18.0-mig-stock_inventory_valuation_report
Open

[18.0][MIG] stock_inventory_valuation_report#472
oscars8a wants to merge 19 commits into
OCA:18.0from
oscars8a:18.0-mig-stock_inventory_valuation_report

Conversation

@oscars8a

Copy link
Copy Markdown

No description provided.

@oscars8a oscars8a changed the title 18.0 mig stock inventory valuation report [18.0] [MIG] stock inventory valuation report Mar 11, 2026
@oscars8a oscars8a changed the title [18.0] [MIG] stock inventory valuation report [18.0][MIG] stock inventory valuation report Mar 11, 2026
@oscars8a oscars8a force-pushed the 18.0-mig-stock_inventory_valuation_report branch from f961621 to 8770977 Compare March 18, 2026 13:08
@rrebollo

Copy link
Copy Markdown

@oscars8a please respect the commit history. Review the wiki documentation or other resources to ensure the migration is done properly. https://github.com/OCA/maintainer-tools/wiki/Migration-to-version-18.0

@oscars8a oscars8a force-pushed the 18.0-mig-stock_inventory_valuation_report branch 3 times, most recently from e726df3 to 9a84ee8 Compare March 26, 2026 12:16
@rrebollo

Copy link
Copy Markdown

@oscars8a When I mentioned commit history, I was referring to past versions. What we have now is essentially the history of this migration—which, by the way, you should squash (get rid of).

Here's another resource to help with migrations: https://github.com/OCA/oca-port. Be sure to check out the video tutorials about it from OCA Days events on YouTube.

ps-tubtim and others added 18 commits March 26, 2026 13:30
Currently translated at 100.0% (39 of 39 strings)

Translation: stock-logistics-reporting-12.0/stock-logistics-reporting-12.0-stock_inventory_valuation_report
Translate-URL: https://translation.odoo-community.org/projects/stock-logistics-reporting-12-0/stock-logistics-reporting-12-0-stock_inventory_valuation_report/it/
Co-authored-by: dessanhemrayev <dessanhemrayev@gmail.com>
Co-authored-by: Alessandro Uffreduzzi <alessandro.uffreduzzi@pytech.it>
Currently translated at 100.0% (39 of 39 strings)

Translation: stock-logistics-reporting-14.0/stock-logistics-reporting-14.0-stock_inventory_valuation_report
Translate-URL: https://translation.odoo-community.org/projects/stock-logistics-reporting-14-0/stock-logistics-reporting-14-0-stock_inventory_valuation_report/it/
@oscars8a oscars8a force-pushed the 18.0-mig-stock_inventory_valuation_report branch 2 times, most recently from 8db58c7 to 01fcf5e Compare March 26, 2026 12:36
@oscars8a

Copy link
Copy Markdown
Author

Thank you very much for your help, @rrebollo

@rrebollo

rrebollo commented Mar 26, 2026

Copy link
Copy Markdown

The title of this PR should be something along the lines of:

[18.0][MIG] stock_inventory_valuation_report

In the description, you should mention the corresponding issue where migrations for the target version are listed. This ensures all contributors can see you've stepped in to handle the migration and helps avoid duplicate work, in this case: #346.

I also noticed this addon hasn't been migrated since version 14.0. Typically, that would suggest it's no longer needed—either because its functionality was merged into the built-in modules or because another OCA addon now provides the same feature.

Did your research consider that possibility?

Comment thread eslint.config.cjs

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

this file shouldn't be part of the migration

Copy link
Copy Markdown
Author

Choose a reason for hiding this comment

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

Yes, but it’s the only way I’ve seen to fix the precommit, https://github.com/OCA/stock-logistics-reporting/actions/runs/23599897085/job/68726677853?pr=472

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

The real problem could be linked to the copier repo template—maybe it needs to be updated. Remove the file and leave a message explaining that the failing test is unrelated to the addon migration.

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

Had you tried git pull --rebase origin 18.0?

Copy link
Copy Markdown
Author

Choose a reason for hiding this comment

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

It's up to date; I updated it two weeks ago. The problem was specifying the file as an ESM module.

Comment thread stock_inventory_valuation_report/wizard/stock_quantity_history.py Outdated
Comment thread stock_inventory_valuation_report/wizard/stock_quantity_history.py Outdated
@oscars8a oscars8a changed the title [18.0][MIG] stock inventory valuation report [18.0][MIG] stock_inventory_valuation_report Mar 26, 2026
@oscars8a oscars8a mentioned this pull request Mar 26, 2026
23 tasks
@oscars8a oscars8a force-pushed the 18.0-mig-stock_inventory_valuation_report branch from 6463555 to 48eddd5 Compare March 26, 2026 14:30

@rrebollo rrebollo left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

Code Review: Great work! The code looks good to me (LGTM). Thank you for your contribution! I've provided a few suggestions for your consideration—feel free to address them as you see fit.

For example you could replace deprecated t-esc by t-out in some templates.

from odoo.tools import mute_logger, test_reports


class TestStockInventoryValuation(common.TransactionCase):

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

Use BaseCommon instead. You can check out the source code and adopt it if the benefits apply.

Copy link
Copy Markdown
Author

Choose a reason for hiding this comment

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

Thank you very much for your help and patience

@JorgeQuinteros JorgeQuinteros left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

LGTM Functional Review.

@OCA-git-bot

Copy link
Copy Markdown
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). 🤖

@BhaveshHeliconia BhaveshHeliconia left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Image

Could you please squash these two commits into the migration commit?

@oscars8a oscars8a force-pushed the 18.0-mig-stock_inventory_valuation_report branch from a0f358a to 99d3610 Compare May 7, 2026 14:01
@OCA-git-bot OCA-git-bot added series:18.0 mod:stock_inventory_valuation_report Module stock_inventory_valuation_report labels May 7, 2026
@oscars8a oscars8a requested a review from BhaveshHeliconia May 7, 2026 14:14
@oscars8a

oscars8a commented May 7, 2026

Copy link
Copy Markdown
Author
Image Could you please squash these two commits into the migration commit?

Thanks, you've already done it. Best regards,

@OCA-git-bot

Copy link
Copy Markdown
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). 🤖

2 similar comments
@OCA-git-bot

Copy link
Copy Markdown
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). 🤖

@OCA-git-bot

Copy link
Copy Markdown
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). 🤖

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.