[17.0][IMP] product_contract: Add contract configurator instead of making tree not editable #1120
Conversation
|
Hi @sbejaoui, |
carolinafernandez-tecnativa
changed the title
|
ping @pedrobaeza |
There was a problem hiding this comment.
I'm not finding the corresponding fields in the sales order line tree:
Anyway, seeing how Odoo has adapted other similar needs, where we need extra info for a sales order line, showing a popup with that extra details, I think it should be the way to go:
It seems not very complicated to be added, taking event_sale as example:
https://github.com/odoo/odoo/tree/17.0/addons/event_sale/static/src/js
@CarlosRoca13 can you please take this and perform it, as you already knows JS and the event_sale controller?
@sbejaoui do you agree?
Another question: why not doing a detailed_type='contract' instead of having a special field is_contract at product level?
| <field | ||
| name="contract_id" | ||
| options='{"no_create": True}' | ||
| column_invisible="parent.is_contract == False" |
There was a problem hiding this comment.
Shorter, but also more correct, as is_contract may be other value than False, but also a falsy value.
| column_invisible="parent.is_contract == False" | |
| column_invisible="not parent.is_contract" |
Do the same on the rest.
| <field | ||
| name="date_start" | ||
| column_invisible="parent.is_contract == False" | ||
| required="is_contract == True" |
There was a problem hiding this comment.
| required="is_contract == True" | |
| required="is_contract" |
and the same for the rest of the occurrences.
|
@pedrobaeza @carolinafernandez-tecnativa pushed changes to add the product contract configurator like odoo does for events |
|
@pedrobaeza LGTM, what do you think? |
|
The PR title is incorrect, and I think the first commit is now not needed if it has to be undo in the second one. |
There was a problem hiding this comment.
About the functionality, this sounds better, although the "Contract" and "Contract line recurrency" should be at the end in a separate section, as this a very special case. I'm also surprised that the "End date" is filled by default, where it shouldn't.
And what about adding some demo master data to ease the tests?
…ree not editable Before this changes, when trying to edit a line of sale order, it was opening the form of the line. But following the way to work of odoo with sale event, we have make a new contract configurator that will be opened when selecting a product of type contract.
CarlosRoca13
force-pushed
the
17.0-imp-product_contract-order-lines-view
branch
from
01f8e1e
to
88d2cb9
Compare
pedrobaeza
changed the title
CarlosRoca13
force-pushed
the
17.0-imp-product_contract-order-lines-view
branch
from
f96a46e
to
51a855b
Compare
| ("1", "1"), | ||
| ("2", "2"), | ||
| ("3", "3"), |
There was a problem hiding this comment.
| ("1", "1"), | |
| ("2", "2"), | |
| ("3", "3"), | |
| ("1", "First month"), | |
| ("2", "Second month"), | |
| ("3", "Third month"), |
| ("2", "2"), | ||
| ("3", "3"), | ||
| ], | ||
| "Force Month", |
There was a problem hiding this comment.
| "Force Month", | |
| "Force Month", | |
| help="Force the month to be used inside the quarter", |
| ("1", "1"), | ||
| ("2", "2"), | ||
| ("3", "3"), | ||
| ("4", "4"), | ||
| ("5", "5"), | ||
| ("6", "6"), |
There was a problem hiding this comment.
The same here about the labels and the help.
| + relativedelta(day=1, months=1) | ||
| - relativedelta(days=1) |
There was a problem hiding this comment.
Does this work?
| + relativedelta(day=1, months=1) | |
| - relativedelta(days=1) | |
| + relativedelta(day=1, months=1, days=-1) |
| increment = month_nb if not forced_month else 0 | ||
| start_date = ( | ||
| start_date | ||
| + relativedelta(months=1 + increment) |
There was a problem hiding this comment.
Isn't this missing?
| + relativedelta(months=1 + increment) | |
| + relativedelta(months=1 + increment, day=1) |
There was a problem hiding this comment.
Already established in the above definition https://github.com/OCA/contract/pull/1120/files#diff-308d8794eb63f4f473a8ce812eba0aacdbdf86a8c0c8f9c3786f1de1baade116R317-R319
product_contract/readme/ROADMAP.md
Outdated
| Is not added support for dates computed from confirmation date of sale order for | ||
| recurrent types daily, weekly or monthlylastday |
There was a problem hiding this comment.
| Is not added support for dates computed from confirmation date of sale order for | |
| recurrent types daily, weekly or monthlylastday | |
| - There's no support right now for computing the start date for the | |
| following recurrent types: daily, weekly and monthlylastday. |
| today = fields.Date.today() | ||
| month_period = month = today.month | ||
| month_nb = MONTH_NB_MAPPING[line.recurring_rule_type] | ||
| # The period number is started by 0 to be able calculate the month |
There was a problem hiding this comment.
| # The period number is started by 0 to be able calculate the month | |
| # The period number is started by 0 to be able to calculate the month |
| string="Contract Template", | ||
| compute="_compute_contract_template_id", | ||
| ) | ||
| recurring_rule_type = fields.Selection( |
There was a problem hiding this comment.
Now that we have configured all the start date thing in the product, it doesn't seem OK to also have again this configuration here, don't you think?
There was a problem hiding this comment.
I think we should keep it in case the product is set to manual mode, but the unnecessary fields should be hidden in the view.
There was a problem hiding this comment.
@sbejaoui which was the reason to define a recurrency in the product and ask for that recurrency again in the sales order line? Can we strip that part for simplifying code and UI?
CarlosRoca13
force-pushed
the
17.0-imp-product_contract-order-lines-view
branch
from
51a855b
to
914dbd2
Compare
|
Changes done. Let me add some tests for this new feature |
CarlosRoca13
force-pushed
the
17.0-imp-product_contract-order-lines-view
branch
from
914dbd2
to
4b5766d
Compare
|
Tests added |
CarlosRoca13
force-pushed
the
17.0-imp-product_contract-order-lines-view
branch
from
4b5766d
to
62b1a94
Compare
There was a problem hiding this comment.
Seems to work OK, but I find something weird, as if you add any line in the contract template, when confirming the quotation, both the sales order line and the template lines are added in the resulting contract. Was it that way in the original 12.0 implementation?
Something that may be related to this PR is that the extras lines don't have the proper start date.
|
Ah, another thing: I think that the extra description about the recurrency and so on shouldn't be transferred to the contract line. Maybe you can add some markers at the beginning and the end to easily recognize it and remove it when creating the contract line. |
Yes, when the template has a line that line is added to the contract.
What date should be set to that lines?
Ok, I'll let you know when it's ready. |
…using confirmation date_start With these changes, we allow the contract line start date to be computed using the order confirmation date. When the product is configured with any of the options set in contract_start_date_method other than manual, the start date will be calculated based on the established date and the selected period. Additionally, we can force the month in which we will work in case the frequency is yearly, quarterly, or semesterly. Is not added support for daily, weekly or monthlylastday in this commit.
CarlosRoca13
force-pushed
the
17.0-imp-product_contract-order-lines-view
branch
from
62b1a94
to
9e81292
Compare
|
Change done |
This comment was marked as resolved.
This comment was marked as resolved.
|
On my way to merge this fine PR! |
|
Congratulations, your PR was merged at 64b9d09. Thanks a lot for contributing to OCA. ❤️ |
Before these changes, when trying to edit a line of sale order, it was opening the form of the line. But following the Odoo way of working with the events sales, we have make a new contract configurator that will be opened in a popup when selecting a contract product.
@Tecnativa TT50535