-
Notifications
You must be signed in to change notification settings - Fork 118
[IMP] Add delivery date 5867760 #4995
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: 19.0-l10n_es-accounting-onboarding-malb
Are you sure you want to change the base?
[IMP] Add delivery date 5867760 #4995
Conversation
|
This PR targets the un-managed branch odoo-dev/odoo:19.0-l10n_es-accounting-onboarding-malb, it needs to be retargeted before it can be merged. |
malb-odoo
left a comment
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 for the pr 😄 Commit doesn't follow the guidelines btwww
https://www.odoo.com/documentation/19.0/contributing/development/git_guidelines.html
| for move in self: | ||
| super()._compute_delivery_date() | ||
| if move.country_code == 'ES' and move.state == 'posted' and move.invoice_date and not move.delivery_date: | ||
| move.delivery_date == move.invoice_date No newline at end of file |
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.
You can set up you IDE to add an empty line at the end, otherwise you have that everytime ahah
| for move in self: | ||
| if move.country_code == 'ES': | ||
| move.show_delivery_date = move.is_sale_document() | ||
| else: | ||
| move.show_delivery_date = move.delivery_date and move.is_sale_document() |
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.
Use a super before the loop so that you don't need the else 😄
| @api.depends('state') | ||
| def _compute_delivery_date(self): | ||
| for move in self: | ||
| super()._compute_delivery_date() |
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.
Move the super on top of the loop otherwise you could have a looot of calls to the super
| <template id="external_layout_standard" inherit_id="web.external_layout_standard"> | ||
| <xpath expr="//div[@name='company_address']" position="after"> | ||
| <t t-if="o._name == 'account.move'"> | ||
| <div name="delivery_date"> | ||
| <p>Delivery Date: <span t-out="o.delivery_date" /></p> | ||
| </div> | ||
| </t> | ||
| </xpath> | ||
| </template> | ||
| </odoo> No newline at end of file |
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.
Actually this was not needed, the pdf is already good (it was my bad 👀)
cbc5cd4 to
b12cab2
Compare
b12cab2 to
6d98a43
Compare
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.
Few thingssss, also still commit message can be improved 😄
|
|
||
|
|
||
| @api.depends('state') | ||
| def _compute_delivery_date(self): |
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.
Fine to do it like this but i would expect a extend of the action_post or _post function instead. Here if you do the compute, anyfield that is used in the compute should be in the depends, soooo it means it will be triggered anytime the country code, state, invoice_date is changed which is not really what we want 😄
| for move in self: | ||
| if move.country_code == 'ES' and move.state == 'posted' and move.invoice_date and not move.delivery_date: | ||
| move.delivery_date == move.invoice_date | ||
|
No newline at end of file |
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.
Weird thing here ahaha, you should have an empty line at the end of the file without space to remove the red circle, so i think you have a tab space in line 48 so if you remove it it should be good
| if move.country_code == 'ES': | ||
| move.show_delivery_date = move.is_sale_document() | ||
|
|
||
|
|
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.
One extra line that can be removed
| <xpath expr="//div[@id='partner_vat_address_not_same_as_shipping']" position="after"> | ||
| <div t-if="o.partner_id.company_registry and o.company_id.country_code == 'ES'"> | ||
| ID: <span t-field="o.partner_id.company_registry"/> | ||
| <div t-if="o.partner_id.company_registry and o.company_id.country_code == 'ES'"> ID: <span | ||
| t-field="o.partner_id.company_registry" /> | ||
| </div> | ||
| </xpath> | ||
|
|
||
| <xpath expr="//div[@id='partner_vat_address_same_as_shipping']" position="after"> | ||
| <div t-if="o.partner_id.company_registry and o.company_id.country_code == 'ES'"> | ||
| ID: <span t-field="o.partner_id.company_registry"/> | ||
| <div t-if="o.partner_id.company_registry and o.company_id.country_code == 'ES'"> ID: <span | ||
| t-field="o.partner_id.company_registry" /> | ||
| </div> | ||
| </xpath> | ||
|
|
||
| <xpath expr="//div[@id='partner_vat_no_shipping']" position="after"> | ||
| <div t-if="o.partner_id.company_registry and o.company_id.country_code == 'ES'"> | ||
| ID: <span t-field="o.partner_id.company_registry"/> | ||
| <div t-if="o.partner_id.company_registry and o.company_id.country_code == 'ES'"> ID: <span | ||
| t-field="o.partner_id.company_registry" /> | ||
| </div> |
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.
Those diffs are not necessary, linter issue ?
Description of the issue/feature this PR addresses:
Add Delivery date to invoice if the company is spanish
Current behavior before PR:
The Delivery date didn't appear always and sometimes it was not specified.
Desired behavior after PR is merged:
It will always now be specified if it is a spanish company.
Therefore, it will always appear in the invoice.
I confirm I have signed the CLA and read the PR guidelines at www.odoo.com/submit-pr