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

[17.0][MIG] pos_partner_firstname: Migration to 17.0 #1126

Merged
merged 49 commits into from
Jan 24, 2024

Conversation

etobella
Copy link
Member

Refactoring to adapt to OWL 2.0

Changes applied to avoid an extra connection to server

robyf70 and others added 30 commits January 10, 2024 11:52
Currently translated at 100.0% (4 of 4 strings)

Translation: pos-12.0/pos-12.0-pos_partner_firstname
Translate-URL: https://translation.odoo-community.org/projects/pos-12-0/pos-12-0-pos_partner_firstname/ca/
Currently translated at 100.0% (4 of 4 strings)

Translation: pos-12.0/pos-12.0-pos_partner_firstname
Translate-URL: https://translation.odoo-community.org/projects/pos-12-0/pos-12-0-pos_partner_firstname/es/
Updated by "Update PO files to match POT (msgmerge)" hook in Weblate.

Translation: pos-12.0/pos-12.0-pos_partner_firstname
Translate-URL: https://translation.odoo-community.org/projects/pos-12-0/pos-12-0-pos_partner_firstname/
Currently translated at 100.0% (4 of 4 strings)

Translation: pos-12.0/pos-12.0-pos_partner_firstname
Translate-URL: https://translation.odoo-community.org/projects/pos-12-0/pos-12-0-pos_partner_firstname/it/
…es_order from POS frontend

Otherwise they would get access error on ir.config_parameter

Steps:

 - Create a user POS manager without administration rights
 - Using that user, open POS frontend
Currently translated at 100.0% (4 of 4 strings)

Translation: pos-12.0/pos-12.0-pos_partner_firstname
Translate-URL: https://translation.odoo-community.org/projects/pos-12-0/pos-12-0-pos_partner_firstname/ca/
Currently translated at 100.0% (4 of 4 strings)

Translation: pos-12.0/pos-12.0-pos_partner_firstname
Translate-URL: https://translation.odoo-community.org/projects/pos-12-0/pos-12-0-pos_partner_firstname/es/
Currently translated at 100.0% (4 of 4 strings)

Translation: pos-12.0/pos-12.0-pos_partner_firstname
Translate-URL: https://translation.odoo-community.org/projects/pos-12-0/pos-12-0-pos_partner_firstname/fr_CH/
Currently translated at 100.0% (4 of 4 strings)

Translation: pos-12.0/pos-12.0-pos_partner_firstname
Translate-URL: https://translation.odoo-community.org/projects/pos-12-0/pos-12-0-pos_partner_firstname/fr_CH/
Currently translated at 100.0% (4 of 4 strings)

Translation: pos-12.0/pos-12.0-pos_partner_firstname
Translate-URL: https://translation.odoo-community.org/projects/pos-12-0/pos-12-0-pos_partner_firstname/fr_CH/
Updated by "Update PO files to match POT (msgmerge)" hook in Weblate.

Translation: pos-12.0/pos-12.0-pos_partner_firstname
Translate-URL: https://translation.odoo-community.org/projects/pos-12-0/pos-12-0-pos_partner_firstname/
Updated by "Update PO files to match POT (msgmerge)" hook in Weblate.

Translation: pos-12.0/pos-12.0-pos_partner_firstname
Translate-URL: https://translation.odoo-community.org/projects/pos-12-0/pos-12-0-pos_partner_firstname/
Alexis de Lattre and others added 2 commits January 10, 2024 11:52
@flotho
Copy link
Member

flotho commented Jan 10, 2024

Hi @etobella , thanks for this.
Did you have a look to #1122 ?
Regards

@etobella
Copy link
Member Author

Yes I checked it, but at the end I made it in my own way using their comments, as the approach was much better 😄

@flotho
Copy link
Member

flotho commented Jan 10, 2024

Hum thanks @etobella ,
@DorianMAG , could you have a look to the @etobella proposal ?

@etobella
Copy link
Member Author

Also, I removed a connection to the server that was unnecessary 😄

Copy link
Contributor

@cvinh cvinh left a comment

Choose a reason for hiding this comment

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

If we want to add a new partner with only firstname or lastname (not both)
We have an error
TypeError: this.showPopup is not a function
at PartnerDetailsEdit.saveChanges

@etobella etobella force-pushed the 17.0-mig-pos_partner_firstname branch from 46ba113 to f25d6df Compare January 12, 2024 10:16
@etobella
Copy link
Member Author

It works now. Thanks for the review!

@etobella
Copy link
Member Author

Actually, it has no sense to require both on PoS but only one on backend, I will keep the logic of backend.

One option would be to keep the same logic using a configuration parameter on backend and frontend. WDYT?

@etobella etobella force-pushed the 17.0-mig-pos_partner_firstname branch from f25d6df to a46a35e Compare January 15, 2024 10:41
@cvinh
Copy link
Contributor

cvinh commented Jan 15, 2024

Actually, it has no sense to require both on PoS but only one on backend, I will keep the logic of backend.

One option would be to keep the same logic using a configuration parameter on backend and frontend. WDYT?

I agree. BTW in v16, firstname AND lastname are mandatory... @robyf70 do you know if it was a requirement for anyone ?
@alexis-via

@robyf70
Copy link
Contributor

robyf70 commented Jan 15, 2024

Actually, it has no sense to require both on PoS but only one on backend, I will keep the logic of backend.
One option would be to keep the same logic using a configuration parameter on backend and frontend. WDYT?

I agree. BTW in v16, firstname AND lastname are mandatory... @robyf70 do you know if it was a requirement for anyone ? @alexis-via

This module was done to give the possibility to create a contact as person from the PoS as well

return this.popup.add(ErrorPopup, {
title: _t("Missing information"),
body: _t("Customer firstname or lastname are required"),
});
Copy link
Contributor

Choose a reason for hiding this comment

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

It should be 'Customer firstname or lastname is required'

@cvinh
Copy link
Contributor

cvinh commented Jan 15, 2024

Actually, it has no sense to require both on PoS but only one on backend, I will keep the logic of backend.
One option would be to keep the same logic using a configuration parameter on backend and frontend. WDYT?

I agree. BTW in v16, firstname AND lastname are mandatory... @robyf70 do you know if it was a requirement for anyone ? @alexis-via

This module was done to give the possibility to create a contact as person from the PoS as well

Thanks @robyf70 we noticed that firstname AND lastname were mandatory in previous versions. The idea is to follow partner_firstname logical which allows partner with firstname OR lastname
@etobella if there's a configuration parameter then it's logical to add it in partner_firstname and inherit it
IMHO I'm approving as it is

Refactoring to OWL 2.0
Disabling an extra connection to server
@etobella etobella force-pushed the 17.0-mig-pos_partner_firstname branch from a46a35e to 3ba9a89 Compare January 17, 2024 16:41
@etobella
Copy link
Member Author

Well, adding the constrain might seem a good idea, but we must remember that some countries do not have lastname, so it makes sense to not make it required

Copy link
Contributor

@cvinh cvinh left a comment

Choose a reason for hiding this comment

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

Functional test ok

@etobella
Copy link
Member Author

/ocabot merge nobump

@OCA-git-bot
Copy link
Contributor

What a great day to merge this nice PR. Let's do it!
Prepared branch 17.0-ocabot-merge-pr-1126-by-etobella-bump-nobump, awaiting test results.

@OCA-git-bot OCA-git-bot merged commit da6e6a6 into OCA:17.0 Jan 24, 2024
5 checks passed
@OCA-git-bot
Copy link
Contributor

Congratulations, your PR was merged at 712663c. 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.