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

Prevent setting duplicate order status names #23142

Merged
merged 4 commits into from
Feb 24, 2021

Conversation

Progi1984
Copy link
Member

@Progi1984 Progi1984 commented Feb 4, 2021

Questions Answers
Branch? 1.7.7.x
Description? Check if an order status has the same name in Add/Edit
Type? bug fix
Category? BO
BC breaks? no
Deprecations? no
Fixed ticket? Fixes #22870
How to test? Add or Edit an order status and try to set the label identical to an existing one (like @hibatallahAouadni did in #22870 (comment))
Specs from @MatShir : #22870 (comment)

This change is Reviewable

@Progi1984 Progi1984 requested a review from a team as a code owner February 4, 2021 16:11
@prestonBot prestonBot added 1.7.7.x Branch Bug Type: Bug labels Feb 4, 2021
@Progi1984 Progi1984 added this to the 1.7.7.2 milestone Feb 4, 2021
@prestonBot prestonBot added the Waiting for wording Status: action required, waiting for wording label Feb 4, 2021
@Julievrz Julievrz added Wording ✔️ Status: check done, wording approved and removed Waiting for wording Status: action required, waiting for wording labels Feb 5, 2021
Copy link
Member

@eternoendless eternoendless left a comment

Choose a reason for hiding this comment

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

Please avoid adding more SQL in controllers

Copy link
Contributor

@PierreRambaud PierreRambaud left a comment

Choose a reason for hiding this comment

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

As Pablo's says, you have to put logic inside ObjectModel

@Progi1984 Progi1984 added the Waiting for author Status: action required, waiting for author feedback label Feb 9, 2021
@prestonBot prestonBot added Waiting for wording Status: action required, waiting for wording and removed Wording ✔️ Status: check done, wording approved labels Feb 9, 2021
@Progi1984 Progi1984 added Wording ✔️ Status: check done, wording approved and removed Waiting for author Status: action required, waiting for author feedback Waiting for wording Status: action required, waiting for wording labels Feb 9, 2021
classes/order/OrderState.php Outdated Show resolved Hide resolved
controllers/admin/AdminStatusesController.php Outdated Show resolved Hide resolved
@Progi1984 Progi1984 requested a review from matks February 9, 2021 16:59
@prestonBot prestonBot added the Waiting for wording Status: action required, waiting for wording label Feb 9, 2021
@sowbiba
Copy link
Contributor

sowbiba commented Feb 22, 2021

waiting for checks to label it waiting for QA

@Progi1984 Progi1984 added the Waiting for QA Status: action required, waiting for test feedback label Feb 23, 2021
@khouloudbelguith
Copy link
Contributor

Hi @Progi1984,

I have an issue following these steps:

  1. Install shop with the English language & France country
  2. Go to BO > Shop Parameters > Order Settings > Statuses Tab
  3. Add new statuses with this name En attente du paiement par chèque
    image
  4. Save
  5. Go to Your profile > change the language of the super admin to French
  6. Go to BO > Orders > Order list page
  7. See error
    image
  8. Go to BO > Shop Parameters > Order Settings > Statuses Tab: you have two statuses with the same name
    image
  9. Try to delete the last status created
  10. See error
    image

PS: this issue is reproduced only after the last fix, previously it was ok
Thank you!

@khouloudbelguith khouloudbelguith added Waiting for author Status: action required, waiting for author feedback and removed Waiting for QA Status: action required, waiting for test feedback labels Feb 23, 2021
@matks matks removed the Waiting for author Status: action required, waiting for author feedback label Feb 23, 2021
@jolelievre jolelievre added the Waiting for QA Status: action required, waiting for test feedback label Feb 23, 2021
@khouloudbelguith
Copy link
Contributor

Hi @Progi1984,

It is ok ✔️
https://drive.google.com/file/d/106x8N8ya1qALb4c0NhDtUUFUXn5iy4NC/view?usp=sharing

I just found new trivial regression but not related to this PR: #23379

Thank you!

@khouloudbelguith khouloudbelguith added QA ✔️ Status: check done, code approved and removed Waiting for QA Status: action required, waiting for test feedback labels Feb 24, 2021
@Progi1984 Progi1984 merged commit fc1e17c into PrestaShop:1.7.7.x Feb 24, 2021
@Progi1984
Copy link
Member Author

Thanks @khouloudbelguith

@Progi1984 Progi1984 deleted the issue22870 branch February 24, 2021 09:57
@eternoendless eternoendless changed the title Check if an order status has the same name in Add/Edit Prevent setting duplicate order status names Mar 22, 2021
@prestonBot prestonBot added the Bug fix Type: Bug fix label Mar 22, 2021
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
1.7.7.x Branch Bug fix Type: Bug fix Bug Type: Bug QA ✔️ Status: check done, code approved Wording ✔️ Status: check done, wording approved
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Having two order states with the same name makes status name disappear in order list after order validation