[15.0][ADD] subscription_oca #971
Conversation
|
@OCA/project-service-maintainers this looks like a clean implementation as we're really struggling to handle customers' requests through the contract_ modules. What do you think? |
|
|
||
| @api.model | ||
| def create(self, values): | ||
| return super().create(values) |
There was a problem hiding this comment.
I think you need to remove unused inheritance.
| return super().create(values) | ||
|
|
||
| def write(self, values): | ||
| return super().write(values) |
There was a problem hiding this comment.
I think you need to remove unused inheritance.
| def get_next_interval(self, type_interval, interval): | ||
| date_start = date.today() | ||
| if type_interval == "days": | ||
| date_start += relativedelta(days=+interval) |
There was a problem hiding this comment.
May be its better to write something like this for all cases:
date_start += relativedelta(**{ type_interval: interval })
|
|
||
| @staticmethod | ||
| def string2datetime(string_date): | ||
| return datetime.strptime(string_date, ODOO_DATE_FORMAT) |
There was a problem hiding this comment.
For this operations you can use builtin to Date field methods
https://www.odoo.com/documentation/16.0/developer/reference/backend/orm.html#date-time-fields
| ) | ||
| elif self.template_id.recurring_rule_type == "years": | ||
| self.recurring_next_date = start_date + relativedelta( | ||
| years=+int(self.template_id.recurring_interval) |
There was a problem hiding this comment.
You can use previous example for such case
| elif self.template_id.recurring_rule_type == "months": | ||
| date_string += relativedelta(months=+self.template_id.recurring_interval) | ||
| else: | ||
| date_string += relativedelta(years=+self.template_id.recurring_interval) |
There was a problem hiding this comment.
You can use previous example to make code shorter
| else: | ||
| self.calculate_recurring_next_date(today) | ||
|
|
||
| def calculate_recurring_next_date(self, start_date): |
There was a problem hiding this comment.
@carlos-domatix For these kind of computations, have you looked at https://github.com/OCA/server-ux/tree/13.0/base_recurrence ?
That one could be useful to lower the amount of code here
There was a problem hiding this comment.
@rousseldenis any chance to migrate it to 15? :)
There was a problem hiding this comment.
@francesco-ooops Feel free to do it. It should be quite easy. But no time to do it now.
There was a problem hiding this comment.
@carlos-domatix FYI we've migrated base_recurrence here, you can add a dependency to that PR to employ it here
There was a problem hiding this comment.
Hi @carlos-domatix , just to say the migration is complete :)
|
@carlos-domatix we'd like to get this merged soon so let us know if we can help in any way or pick up the task in case you don't have time to work on it ATM Thank you! |
I agree. Quite 😢 in the change of contract in v12 that broke a simple ux module from Odoo 8 #207 👍🏼 Thank you! |
|
Thanks for the fixes and suggestions! I think I'll have some time next week to get all the changes done. |
@carlos-domatix any good news ? :) |
carlos-domatix
force-pushed
the
15.0-add-subscription_oca
branch
2 times, most recently
from
68a14dd
to
352b39d
Compare
|
Changes made, except for the dependency on base_recurrence because using this module would result in losing functionality, such as the ability to bill every 15 days. You can make this change yourselves if you consider it necessary for the merge since I won't have much time available from now on. |
|
@rousseldenis good for you? |
|
@OCA/project-service-maintainers could this be merged? it can be later improved to include dependency to base_recurrence if needed, but we'd like to start migration to v14 asap :) |
@rafaelbn good for you? |
| date_start += relativedelta(**{type_interval: interval}) | ||
| return date_start | ||
|
|
||
| def create_subscription(self, lines, template_id): |
There was a problem hiding this comment.
| def create_subscription(self, lines, template_id): | |
| def create_subscription(self, lines, subscription_tmpl): |
| subscription_lines.append((0, 0, line.get_subscription_line_values())) | ||
|
|
||
| if template_id: | ||
| sub_id = self.env["sale.subscription"].create( |
There was a problem hiding this comment.
| sub_id = self.env["sale.subscription"].create( | |
| rec [or sub] = self.env["sale.subscription"].create( |
|
@carlos-domatix if you don't have time to work on this, would you like for us to pick it up and bring it to merge? of course you'll remain the author of the module |
|
I agree with @simahawk about test, It's not mandatory but bery recommended to maintain and prevent errors in the whole repository 🔴 Failing after 1s — 41.92% of diff hit (target 97.46%) |
|
@francesco-ooops , august, so please wait a little bit more time to see if @carlos-domatix answer. Thank you! |
| compute="_compute_account_invoice_ids_count", string="Invoice Count" | ||
| ) | ||
|
|
||
| limit_rule_count = fields.Integer(default=0, string="Subscription limit count") |
There was a problem hiding this comment.
AFAICS this is not used anyhwere...
@carlos-domatix hi, waiting for your feedback in order to merge ASAP :) |
I will try to find a place this week, although I doubt that I will be able to do the tests. |
|
If @carlos-domatix don't know how to make test, and you @francesco-ooops know, let's go! But @simahawk comments should be attended please @carlos-domatix This module remember me to the classic 😄 |
|
@carlos-domatix any good news? |
carlos-domatix
force-pushed
the
15.0-add-subscription_oca
branch
from
352b39d
to
086c1ba
Compare
|
I would say that I have already made all the suggested changes. Sorry but there were many and right now we are quite busy with other matters. On the other hand, we just received some direct comments with concerns about the license. |
|
@simahawk could you please review? |
| <field name="sequence">0</field> | ||
| <field name="type">pre</field> | ||
| <field name="description"> | ||
| DEFAULT STAGE |
There was a problem hiding this comment.
is this "DEFAULT STAGE" wanted?
Also, wouldn't be better to flag them as noupdate?
There was a problem hiding this comment.
Yes, it is the first stage, why should it not exist?
It's inside <data noupdate="1">, I don't know what you mean.
There was a problem hiding this comment.
ahh sorry, I've looked only at the top 😓
is this "DEFAULT STAGE" wanted?
I mean: why do you need this sentence in the description?
There was a problem hiding this comment.
Ok, I delete it
| <field name="name">The subscription is too expensive</field> | ||
| </record> | ||
|
|
||
| <record id="close_reason_expecs" model="sale.subscription.close.reason"> |
There was a problem hiding this comment.
| <record id="close_reason_expecs" model="sale.subscription.close.reason"> | |
| <record id="close_reason_requirement" model="sale.subscription.close.reason"> |
| inverse_name="partner_id", | ||
| string="Subscriptions", | ||
| ) | ||
| subscription_ids_count = fields.Integer( |
There was a problem hiding this comment.
| subscription_ids_count = fields.Integer( | |
| subscription_count = fields.Integer( |
| try: | ||
| subscription.generate_invoice() | ||
| except Exception: | ||
| logger.exception("Error during Subscriptions Management cron") |
There was a problem hiding this comment.
| logger.exception("Error during Subscriptions Management cron") | |
| logger.exception("Error on subscription invoice generate") |
| if self.template_id.invoicing_mode == "invoice_send": | ||
| mail_template_id = self.template_id.invoice_mail_template_id | ||
| invoice_id.with_context(force_send=True).message_post_with_template( | ||
| mail_template_id.id, |
There was a problem hiding this comment.
| mail_template_id.id, | |
| mail_template.id, |
There was a problem hiding this comment.
well, apply to all the vars that are not ids but... records 😉
| def write(self, values): | ||
| res = super().write(values) | ||
| if "stage_id" in values: | ||
| if self.stage_id: |
There was a problem hiding this comment.
this is broken if you have multiple records, no? 🔥
| _description = "Subscription stage" | ||
| _order = "sequence, name, id" | ||
|
|
||
| @api.constrains("type") |
| <notebook> | ||
| <page string="Invoicing"> | ||
| <group> | ||
| <group> |
| sale_subscription_id = self.env["sale.subscription"].search( | ||
| [("id", "=", self.env.context["active_id"])] | ||
| ) |
There was a problem hiding this comment.
why searching when you have it?
| sale_subscription_id = self.env["sale.subscription"].search( | |
| [("id", "=", self.env.context["active_id"])] | |
| ) | |
| sale_subscription = self.env["sale.subscription"].browse(self.env.context["active_id"]) |
|
@simahawk appreciate it! |
|
@simahawk Thanks for the suggestions, I'll do it as soon as possible. |
carlos-domatix
force-pushed
the
15.0-add-subscription_oca
branch
from
086c1ba
to
df2b18b
Compare
carlos-domatix
force-pushed
the
15.0-add-subscription_oca
branch
from
df2b18b
to
382eccb
Compare
|
Changes made, I hope everything is there. |
|
@elvise @francesco-ooops good for you from functional perspective after all these changes? I have no time to test on my side 😉 |
@simahawk LGTM 🫡 |
| template_id = self.env["sale.subscription.template"].search( | ||
| [("id", "=", values["template_id"])] | ||
| ) |
There was a problem hiding this comment.
| template_id = self.env["sale.subscription.template"].search( | |
| [("id", "=", values["template_id"])] | |
| ) | |
| template_id = self.env["sale.subscription.template"].browse(values["template_id"]) |
| self.env["sale.subscription.stage"] | ||
| .search([("type", "=", "pre")], order="sequence desc")[-1] | ||
| .id |
There was a problem hiding this comment.
| self.env["sale.subscription.stage"] | |
| .search([("type", "=", "pre")], order="sequence desc")[-1] | |
| .id | |
| self.env["sale.subscription.stage"] | |
| .search([("type", "=", "pre")], order="sequence desc", limit=1) | |
| .id |
|
/ocabot merge nobump |
|
This PR looks fantastic, let's merge it! |
|
Congratulations, your PR was merged at dd986a0. Thanks a lot for contributing to OCA. ❤️ |
Thanks, I'll have it in mind. |
This module allows creating subscriptions that generate recurring invoices or orders. It also enables the sale of products that generate subscriptions.