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

[14.0] [mig] pos_partner_firstname #775

Merged
merged 31 commits into from
Apr 12, 2022

Conversation

robyf70
Copy link
Contributor

@robyf70 robyf70 commented Apr 1, 2022

Migration to v14.0

@robyf70
Copy link
Contributor Author

robyf70 commented Apr 1, 2022

@legalsylvain @francesco-ooops Ready to merge

Copy link
Contributor

@francesco-ooops francesco-ooops left a comment

Choose a reason for hiding this comment

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

Alright

Copy link
Contributor

@legalsylvain legalsylvain left a comment

Choose a reason for hiding this comment

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

just tested on runboat. a blocking point :

image

step to reproduce

  • open PoS
  • customer
  • "create"

unpossible to create a new customer.

@robyf70
Copy link
Contributor Author

robyf70 commented Apr 1, 2022

just tested on runboat. a blocking point :

image

step to reproduce

  • open PoS
  • customer
  • "create"

unpossible to create a new customer.

Let give it a try

Copy link
Contributor

@francesco-ooops francesco-ooops left a comment

Choose a reason for hiding this comment

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

Yes, I'm seeing other behaviors that are not in line with version 12.0, eg: https://recordit.co/oDqBfR89Dw

STEPS TO REPRODUCE

  • Click on customer > select a customer with type = individual > edit
  • Activate flag "company" > name and last name are moved to field "name" >
  • deselect flag company > write something in field "name" > save

CURRENT BEHAVIOR

first surname is not updated correctly

(sorry for the hasty earlier review)

@robyf70 robyf70 force-pushed the 14.0-mig-pos_partner_firstname branch 3 times, most recently from 09b22a4 to 47b6b70 Compare April 1, 2022 14:42
@robyf70
Copy link
Contributor Author

robyf70 commented Apr 1, 2022

Yes, I'm seeing other behaviors that are not in line with version 12.0, eg: https://recordit.co/oDqBfR89Dw

STEPS TO REPRODUCE

  • Click on customer > select a customer with type = individual > edit
  • Activate flag "company" > name and last name are moved to field "name" >
  • deselect flag company > write something in field "name" > save

CURRENT BEHAVIOR

first surname is not updated correctly

(sorry for the hasty earlier review)

I've cleaned up the code, but the above behaviour depends by the the way how res.partner works.

@robyf70
Copy link
Contributor Author

robyf70 commented Apr 1, 2022

just tested on runboat. a blocking point :

image

step to reproduce

  • open PoS
  • customer
  • "create"

unpossible to create a new customer.

This is fixed

Copy link
Contributor

@francesco-ooops francesco-ooops left a comment

Choose a reason for hiding this comment

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

Still issue with saving new contact: https://recordit.co/1Q4Wd1NJqK

@robyf70 robyf70 force-pushed the 14.0-mig-pos_partner_firstname branch from 47b6b70 to 01cde21 Compare April 1, 2022 15:36
@robyf70
Copy link
Contributor Author

robyf70 commented Apr 1, 2022

Still issue with saving new contact: https://recordit.co/1Q4Wd1NJqK

Ok! This finally fixed

@robyf70 robyf70 force-pushed the 14.0-mig-pos_partner_firstname branch from 01cde21 to a3b3a6e Compare April 1, 2022 15:38
Copy link
Contributor

@francesco-ooops francesco-ooops left a comment

Choose a reason for hiding this comment

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

I wouldn't leave error "raw": https://recordit.co/YLOjYVWsOc

for the rest it looks ok

@rafaelbn
Copy link
Member

rafaelbn commented Apr 1, 2022

/ocabot migration pos_partner_firstname

@OCA-git-bot OCA-git-bot added this to the 14.0 milestone Apr 1, 2022
@OCA-git-bot OCA-git-bot mentioned this pull request Apr 1, 2022
16 tasks
@robyf70 robyf70 force-pushed the 14.0-mig-pos_partner_firstname branch 3 times, most recently from 4815442 to 4f36475 Compare April 2, 2022 08:04
@robyf70
Copy link
Contributor Author

robyf70 commented Apr 2, 2022

I wouldn't leave error "raw": https://recordit.co/YLOjYVWsOc

for the rest it looks ok

I think I've fixed the remaining issue

Copy link
Contributor

@francesco-ooops francesco-ooops left a comment

Choose a reason for hiding this comment

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

https://recordit.co/kbvlUOOsDf

STEPS TO REPRODUCE

create a new company > set as customer > edit record > unflag "company" > click "deselect customer" (do not save)

CURRENT BEHAVIOR

First name appears as "false" in main interface

@robyf70 robyf70 force-pushed the 14.0-mig-pos_partner_firstname branch from 4f36475 to 91a6103 Compare April 4, 2022 09:04
@robyf70
Copy link
Contributor Author

robyf70 commented Apr 4, 2022

https://recordit.co/kbvlUOOsDf

STEPS TO REPRODUCE

create a new company > set as customer > edit record > unflag "company" > click "deselect customer" (do not save)

CURRENT BEHAVIOR

First name appears as "false" in main interface

Ok! Let's try another round and, finger crossed, now works

Copy link
Contributor

@francesco-ooops francesco-ooops left a comment

Choose a reason for hiding this comment

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

@robyf70 LGTM!

@robyf70
Copy link
Contributor Author

robyf70 commented Apr 4, 2022

@robyf70 LGTM!

@legalsylvain Can we merge now?

Copy link
Contributor

@legalsylvain legalsylvain left a comment

Choose a reason for hiding this comment

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

Just tested :

Use case 1 ko :

  • click on create a new partner (in the front office).
  • Set Name & surname

image

  • Then click on "is company"
  • Set a complete name

image

  • Save

Expected values :

  • is_company true
  • name : "Test Complete Name"

Actual values :

  • is_company : True -> OK
  • name "Test Name Test Surname" KO

image

@robyf70 robyf70 force-pushed the 14.0-mig-pos_partner_firstname branch 2 times, most recently from a112b8c to 509fbd5 Compare April 11, 2022 14:23
@robyf70
Copy link
Contributor Author

robyf70 commented Apr 11, 2022

@ivantodorovich I've reviewed your suggestion.

Copy link
Contributor

@ivantodorovich ivantodorovich left a comment

Choose a reason for hiding this comment

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

@legalsylvain have all your remarks been addressed ?

@legalsylvain
Copy link
Contributor

@legalsylvain have all your remarks been addressed ?

Well, yes, in the roadmad section ;-)...

joke aside. @robyf70 : could you change fields labels in the front office ?
in the back office : "Last Name" / "First Name" in that order.
in the front office : "Name" / "Surname". (and it is not in the same order).

Except that blocking point, OK for go ahead. We can improve / fix the other points in another PR.

Thanks @robyf70 for porting this module.

Copy link
Contributor

@legalsylvain legalsylvain left a comment

Choose a reason for hiding this comment

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

LGTM, except the label and order of the field in the front-office.

@robyf70 robyf70 force-pushed the 14.0-mig-pos_partner_firstname branch from 509fbd5 to 6abd777 Compare April 11, 2022 15:41
@robyf70 robyf70 force-pushed the 14.0-mig-pos_partner_firstname branch from 6abd777 to 3e3bff6 Compare April 11, 2022 15:44
@robyf70
Copy link
Contributor Author

robyf70 commented Apr 11, 2022

LGTM, except the label and order of the field in the front-office.

Fixed in last update

@ivantodorovich
Copy link
Contributor

@legalsylvain I let you do the honors if it's ok to you

@legalsylvain
Copy link
Contributor

Just tested on runbot,
when i set first name and last name and try to save, I have the following error :

image

@robyf70 robyf70 force-pushed the 14.0-mig-pos_partner_firstname branch from 3e3bff6 to 4746d25 Compare April 12, 2022 13:57
@robyf70
Copy link
Contributor Author

robyf70 commented Apr 12, 2022

Just tested on runbot, when i set first name and last name and try to save, I have the following error :

image

Hopefully I've sorted out this issue as well.

Copy link
Contributor

@francesco-ooops francesco-ooops left a comment

Choose a reason for hiding this comment

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

LGTM

Copy link
Member

@flotho flotho left a comment

Choose a reason for hiding this comment

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

Tested on runboat
LGTM

@legalsylvain
Copy link
Contributor

thanks !

/ocabot merge patch

@OCA-git-bot
Copy link
Contributor

This PR looks fantastic, let's merge it!
Prepared branch 14.0-ocabot-merge-pr-775-by-legalsylvain-bump-patch, awaiting test results.

@OCA-git-bot OCA-git-bot merged commit 2d8ddf7 into OCA:14.0 Apr 12, 2022
@OCA-git-bot
Copy link
Contributor

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