-
-
Notifications
You must be signed in to change notification settings - Fork 573
[18.0][MIG] Migration of subscription_oca to 18.0 #1304
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
base: 18.0
Are you sure you want to change the base?
Conversation
Currently translated at 95.7% (159 of 166 strings) Translation: contract-16.0/contract-16.0-subscription_oca Translate-URL: https://translation.odoo-community.org/projects/contract-16-0/contract-16-0-subscription_oca/nl/
Updated by "Update PO files to match POT (msgmerge)" hook in Weblate. Translation: contract-17.0/contract-17.0-subscription_oca Translate-URL: https://translation.odoo-community.org/projects/contract-17-0/contract-17-0-subscription_oca/
Currently translated at 100.0% (163 of 163 strings) Translation: contract-17.0/contract-17.0-subscription_oca Translate-URL: https://translation.odoo-community.org/projects/contract-17-0/contract-17-0-subscription_oca/it/
Currently translated at 92.6% (151 of 163 strings) Translation: contract-17.0/contract-17.0-subscription_oca Translate-URL: https://translation.odoo-community.org/projects/contract-17-0/contract-17-0-subscription_oca/fi/
Currently translated at 58.8% (96 of 163 strings) Translation: contract-17.0/contract-17.0-subscription_oca Translate-URL: https://translation.odoo-community.org/projects/contract-17-0/contract-17-0-subscription_oca/tr/
Currently translated at 61.3% (100 of 163 strings) Translation: contract-17.0/contract-17.0-subscription_oca Translate-URL: https://translation.odoo-community.org/projects/contract-17-0/contract-17-0-subscription_oca/tr/
Updated by "Update PO files to match POT (msgmerge)" hook in Weblate. Translation: contract-17.0/contract-17.0-subscription_oca Translate-URL: https://translation.odoo-community.org/projects/contract-17-0/contract-17-0-subscription_oca/
Currently translated at 100.0% (164 of 164 strings) Translation: contract-17.0/contract-17.0-subscription_oca Translate-URL: https://translation.odoo-community.org/projects/contract-17-0/contract-17-0-subscription_oca/it/
Currently translated at 100.0% (164 of 164 strings) Translation: contract-17.0/contract-17.0-subscription_oca Translate-URL: https://translation.odoo-community.org/projects/contract-17-0/contract-17-0-subscription_oca/tr/
Currently translated at 100.0% (164 of 164 strings) Translation: contract-17.0/contract-17.0-subscription_oca Translate-URL: https://translation.odoo-community.org/projects/contract-17-0/contract-17-0-subscription_oca/ca/
Thanks @tarteo, just checked and mostly sorted but one place the currency still shows as missing. |
f80b2b2 to
9cbf302
Compare
|
@chrisandrewmann Fixed! :) |
9cbf302 to
b2f98fb
Compare
|
Thanks @tarteo for the PR (and @chrisandrewmann for the testing)! Does this PR also supersede #1245? |
|
@ivilata Yes |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Thanks @tarteo for the PR! It looks good to me, and I found no issues with manual testing on Runboat.
@chrisandrewmann, if you're also happy with the adaptation, you may want to create a proper positive review so that we can unlock the merging process. Thanks!
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Amazing. Looks good to me, did a quick test of a new subscription, triggered the scheduler to generate invoice and then closed subscription.
|
This PR has the |
1 similar comment
|
This PR has the |
|
@rousseldenis, if the PR looks good to you, would you mind accepting it for meging? Thanks a lot! 🙂 |
|
@tarteo I found a small issue if you have time to fix it? The messages posted to chatter in sale_subscription.py look like this: The solution is 2 lines of code below, works for me. Line 7: from markupsafe import Markup |
b2f98fb to
d182208
Compare
|
@chrisandrewmann Yes, I fixed it! 😄 |
|
@carlos-domatix @carolina-domatix could you please approve to merge this? I have a new PR i'd like to submit which adds automated payments via payment tokens to subscription_oca, something I feel is crucially important for many. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
LGTM, but I don’t have permissions to merge. @rousseldenis could you please review it when you have a moment?
| "website": "https://github.com/OCA/contract", | ||
| "license": "AGPL-3", | ||
| "author": "Domatix, Odoo Community Association (OCA)", | ||
| "author": "Domatix, Onestein, Odoo Community Association (OCA)", |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@tarteo Are you really author? Migration does not mean necessarily you become one.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
See previous PRs on this module. We've put alot of time in this module...
@tarteo I don't see the corresponding commits here |
It is fair to keep original migration commits |
|
@rousseldenis I had to reopen the PR because I did something bad while rebasing, then I just created a separate branch as all commits must be rebased into one commit anyways. |
|
I will not waste anymore time on this, accept it as is or close the PR. If you want to change anything create a new PR copy the changes and merge that with "ACSONE" as author I don't really care... |
|
@tarteo thank you for your contribution! @rousseldenis what are the acceptance criterias for having this merged? Pick all the commits from #1209 and put them in this branch? Are they really needed for the module to work? |
|
Thanks for all your work @tarteo |
|
@chrisandrewmann let's wait until next the week. It there is no feedback please ping me, I will merge it as is. |
|
Hi @rousseldenis would appreciate you looking into this. Would be great to have it merged. |


Continuation of: #1209
Includes:
Apply changes of #1058
Apply changes of #1224
Apply changes of #1210
Apply changes of #1167