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][MIG] sale_invoice_plan, standard migration to 16.0 #2988

Merged
merged 64 commits into from
Dec 8, 2024

Conversation

kittiu
Copy link
Member

@kittiu kittiu commented Mar 2, 2024

Standard migration

@kittiu kittiu changed the title [16.0][MIG] sale_invoice_plan [16.0][MIG] sale_invoice_plan :: Migrate to 16.0 Mar 2, 2024
@kittiu kittiu changed the title [16.0][MIG] sale_invoice_plan :: Migrate to 16.0 [16.0][MIG] sale_invoice_plan, standard migration to 16.0 Mar 2, 2024
@kittiu kittiu force-pushed the 16.0-mig-sale_invoice_plan_kittiu branch 2 times, most recently from aed09d1 to 8804c82 Compare March 3, 2024 04:35
rec.invoice_plan_process = True
continue
rec.invoice_plan_process = False
inv_or_adv = rec.invoice_status == "to invoice" or (
Copy link
Member Author

Choose a reason for hiding this comment

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

@dreispt just FYI, I saw this part of code that you try to refactor on your code is incorrect. This might be the reason that advance is not working?
I try to refactor here too, this will remain the same logic and it works for me.

@rousseldenis
Copy link
Contributor

/ocabot migration sale_invoice_plan

@OCA-git-bot
Copy link
Contributor

The migration issue (#2215) has not been updated to reference the current pull request because a previous pull request (#2438) is not closed.
Perhaps you should check that there is no duplicate work.
CC @MS-OSI

@rousseldenis
Copy link
Contributor

@kittiu What's the status of this ?

Copy link

@DigitalAutomations DigitalAutomations left a comment

Choose a reason for hiding this comment

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

it works

Copy link

@MarcoCalcagni MarcoCalcagni left a comment

Choose a reason for hiding this comment

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

LGTM

is in production

@rvalyi
Copy link
Member

rvalyi commented Dec 4, 2024

/ocabot merge nobump

@OCA-git-bot
Copy link
Contributor

On my way to merge this fine PR!
Prepared branch 16.0-ocabot-merge-pr-2988-by-rvalyi-bump-nobump, awaiting test results.

OCA-git-bot added a commit that referenced this pull request Dec 4, 2024
Signed-off-by rvalyi
@OCA-git-bot
Copy link
Contributor

@rvalyi your merge command was aborted due to failed check(s), which you can inspect on this commit of 16.0-ocabot-merge-pr-2988-by-rvalyi-bump-nobump.

After fixing the problem, you can re-issue a merge command. Please refrain from merging manually as it will most probably make the target branch red.

@rvalyi
Copy link
Member

rvalyi commented Dec 4, 2024

Hum there was an error in the tests during the merge strangely:

2024-12-04 09:44:58,901 562 INFO odoo odoo.tests.suite: ======================================================================
2024-12-04 09:44:58,902 562 ERROR odoo odoo.tests.suite: ERROR: setUpClass (odoo.addons.sale_invoice_plan.tests.test_sale_invoice_plan.TestSaleInvoicePlan)
Traceback (most recent call last):
File "/opt/odoo/odoo/tools/cache.py", line 85, in lookup
r = d[key]
File "", line 2, in getitem
File "/opt/odoo/odoo/tools/func.py", line 87, in locked
return func(inst, *args, **kwargs)
File "/opt/odoo/odoo/tools/lru.py", line 34, in getitem
a = self.d[obj]
KeyError: ('ir.model.data', <function IrModelData._xmlid_lookup at 0x7ff2cc9e1900>, 'account.data_account_type_payable')

During handling of the above exception, another exception occurred:

Traceback (most recent call last):
File "/__w/sale-workflow/sale-workflow/sale_invoice_plan/tests/test_sale_invoice_plan.py", line 26, in setUpClass
user_type_payable = cls.env.ref("account.data_account_type_payable")
File "/opt/odoo/odoo/api.py", line 595, in ref
res_model, res_id = self['ir.model.data']._xmlid_to_res_model_res_id(
File "/opt/odoo/odoo/addons/base/models/ir_model.py", line 2059, in _xmlid_to_res_model_res_id
return self._xmlid_lookup(xmlid)[1:3]
File "", line 2, in _xmlid_lookup
File "/opt/odoo/odoo/tools/cache.py", line 90, in lookup
value = d[key] = self.method(*args, **kwargs)
File "/opt/odoo/odoo/addons/base/models/ir_model.py", line 2052, in _xmlid_lookup
raise ValueError('External ID not found in the system: %s' % xmlid)

cc @kittiu

@kittiu
Copy link
Member Author

kittiu commented Dec 5, 2024

Cc @Saran440 can you help?

@rousseldenis
Copy link
Contributor

@kittiu @rvalyi Two commits should be amended:

image

Could you do something like [IMP] sale_invoice_plan: (...)? Thanks

@rousseldenis
Copy link
Contributor

@kittiu Could you rebase then ?

}

# Create base account to simulate a chart of account
user_type_payable = cls.env.ref("account.data_account_type_payable")
Copy link
Contributor

Choose a reason for hiding this comment

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

Suggested change
user_type_payable = cls.env.ref("account.data_account_type_payable")
user_type_payable = self.company_data['default_account_payable']

Copy link
Member Author

Choose a reason for hiding this comment

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

Thank you @rousseldenis

@kittiu kittiu force-pushed the 16.0-mig-sale_invoice_plan_kittiu branch from 0c34511 to adaaa45 Compare December 6, 2024 03:02
@rvalyi
Copy link
Member

rvalyi commented Dec 6, 2024

@kittiu I think you should have rebased instead of merging the 16.0 branch. Could you fix that? Also, if you could amend your commit message as requested by @rousseldenis , after that I can merge it.

@TheerayutEncoder TheerayutEncoder force-pushed the 16.0-mig-sale_invoice_plan_kittiu branch from 9c9b2cd to bebe744 Compare December 6, 2024 08:23
@@ -153,12 +154,13 @@ def _get_amount_invoice(self, invoices):
amount_invoiced = sum(lines.mapped("price_subtotal"))
return amount_invoiced

@api.depends("invoice_move_ids.state")
Copy link
Contributor

Choose a reason for hiding this comment

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

@kittiu What's the impact of this on existing database in terms of upgrade performance?

Copy link
Member Author

Choose a reason for hiding this comment

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

Really not sure about this. But I doubt that this will be relevant to update the module? Since state is not changing. Do you foresee problem?

@TheerayutEncoder
Copy link
Member

@kittiu @rousseldenis @rvalyi

Commit and test script have fixed.

@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). 🤖

@rvalyi
Copy link
Member

rvalyi commented Dec 8, 2024

/ocabot merge nobump

@OCA-git-bot
Copy link
Contributor

Hey, thanks for contributing! Proceeding to merge this for you.
Prepared branch 16.0-ocabot-merge-pr-2988-by-rvalyi-bump-nobump, awaiting test results.

@OCA-git-bot OCA-git-bot merged commit 5b7bf0c into OCA:16.0 Dec 8, 2024
9 checks passed
@OCA-git-bot
Copy link
Contributor

Congratulations, your PR was merged at 7e08a99. Thanks a lot for contributing to OCA. ❤️

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.