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.

Sign up for GitHub

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

Jump to bottom

Multistore - Payment > Payment methods - Do not display content and update message in all shops or in a group context #27667

Merged
merged 1 commit into from Mar 25, 2022
Merged

Multistore - Payment > Payment methods - Do not display content and update message in all shops or in a group context #27667

merged 1 commit into from Mar 25, 2022

Conversation

Progi1984
Copy link
Contributor

@Progi1984 Progi1984 commented Feb 14, 2022 •
edited by PierreRambaud

Questions Answers
Branch? develop
Description? Multistore - Payment > Payment methods - Do not display content and update message in all shops or in a group context
Type? bug fix
Category? BO
BC breaks? no
Deprecations? no
Fixed ticket? Fixes #19425
How to test? Cf #19425

This change is Reviewable

@Progi1984 Progi1984 added this to the 8.0.0 milestone Feb 14, 2022
@Progi1984 Progi1984 requested a review from a team as a code owner February 14, 2022 15:56
@prestonBot prestonBot added develop Branch Bug fix Type: Bug fix Waiting for wording Status: action required, waiting for wording labels Feb 14, 2022
@matks matks added the migration symfony migration project label Feb 16, 2022
Julievrz
Julievrz previously approved these changes Feb 23, 2022
View reviewed changes
Copy link
Contributor

@Julievrz Julievrz left a comment

Choose a reason for hiding this comment

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

Wording ✔️

@Julievrz Julievrz added Wording ✔️ Status: check done, wording approved and removed Waiting for wording Status: action required, waiting for wording labels Feb 23, 2022
atomiix
atomiix previously approved these changes Feb 24, 2022
View reviewed changes
saulaski
saulaski previously approved these changes Feb 25, 2022
View reviewed changes
@matthieu-rolland matthieu-rolland added the Waiting for QA Status: action required, waiting for test feedback label Feb 28, 2022
@florine2623 florine2623 self-assigned this Mar 2, 2022
@AureRita AureRita self-assigned this Mar 3, 2022
AureRita
AureRita requested changes Mar 3, 2022
View reviewed changes
Copy link
Contributor

@AureRita AureRita left a comment

Choose a reason for hiding this comment

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

Hello @Progi1984 ,

I tested your pr and I noticed the text are correctly changed but the info display (blue alert) is missing, as you can see on this screenshot

i should have the blue alert :
Capture d’écran du 2022-03-03 11-08-58

This is what I' have
Capture d’écran du 2022-03-03 10-48-18

@AureRita AureRita added Waiting for author Status: action required, waiting for author feedback and removed Waiting for QA Status: action required, waiting for test feedback labels Mar 3, 2022
@Progi1984
Copy link
Contributor Author

@AureRita It's a expected behavior as defined here : #19425 (comment)

To avoid bad behaviors and for a better understanding, we should display ONLY the information message in these contexts and update the current message for the following one:
"Note that this page is available in a single shop context only. Switch context to work on it."

@Progi1984 Progi1984 added Waiting for QA Status: action required, waiting for test feedback and removed Waiting for author Status: action required, waiting for author feedback labels Mar 3, 2022
AureRita
AureRita reviewed Mar 3, 2022
View reviewed changes
Copy link
Contributor

@AureRita AureRita left a comment

Choose a reason for hiding this comment

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

If I follow the spec, it's an information message so the block text's color should be blue with the information icon and not with yellow color and the warning icon

@AureRita AureRita added Waiting for author Status: action required, waiting for author feedback and removed Waiting for QA Status: action required, waiting for test feedback labels Mar 3, 2022
@Progi1984
Multistore - Payment > Payment methods - Do not display content and u…
5bd5d03 
…pdate message in all shops or in a group context
@prestonBot prestonBot added Waiting for wording Status: action required, waiting for wording and removed Wording ✔️ Status: check done, wording approved labels Mar 3, 2022
@Progi1984
Copy link
Contributor Author

@AureRita Thanks :)

@atomiix & @matthieu-rolland Changed the alert-warning to alert-info class.

@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 Mar 3, 2022
@Progi1984 Progi1984 added the Waiting for QA Status: action required, waiting for test feedback label Mar 4, 2022
AureRita
AureRita requested changes Mar 7, 2022
View reviewed changes
Copy link
Contributor

@AureRita AureRita left a comment •
edited

Choose a reason for hiding this comment

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

I tested it and the message's type is correctly changed,

as we can see here :

On all shop, We have only the correct info message
image

On groupDefault, the information message is correct, but it's not the only one
image

Hello, @PrestaShop/product-team ,
as you said here : #19425 (comment)
"To avoid bad behaviors and for a better understanding, we should display ONLY the information message in these contexts"
Here, we've two messages but it's not for the same infos, what do we do ?

@AureRita AureRita added Waiting for PM Status: action required, waiting for product feedback and removed Waiting for QA Status: action required, waiting for test feedback labels Mar 7, 2022
@MatShir
Copy link
Contributor

MatShir commented Mar 24, 2022 •
edited

From the spec here https://github.com/PrestaShop/prestashop-specs/blob/master/content/1.7/back-office/multistoregeneralspecs.md and https://github.com/PrestaShop/prestashop-specs/blob/master/content/1.7/broader-topics/multistorespecialsspecs.md, it seems to be the normal behavior if the color of the header is not set for the group shop.

If it is the only bug left, I guess the PR is QA validated @AureRita ?

@MatShir MatShir removed the Waiting for PM Status: action required, waiting for product feedback label Mar 24, 2022
@Progi1984 Progi1984 added the Waiting for QA Status: action required, waiting for test feedback label Mar 25, 2022
AureRita
AureRita approved these changes Mar 25, 2022
View reviewed changes
Copy link
Contributor

@AureRita AureRita left a comment

Choose a reason for hiding this comment

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

hi @MatShir ,

Thank you for your answer, and yes indeed, the PR is QA approved

@AureRita AureRita added QA ✔️ Status: check done, code approved and removed Waiting for QA Status: action required, waiting for test feedback labels Mar 25, 2022
@Progi1984 Progi1984 merged commit 476aa3a into PrestaShop:develop Mar 25, 2022
@Progi1984 Progi1984 deleted the issue19425 branch March 25, 2022 11:00
@Progi1984
Copy link
Contributor Author

Thanks @AureRita @florine2623 @MatShir

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@matthieu-rolland matthieu-rolland matthieu-rolland approved these changes

@atomiix atomiix atomiix approved these changes

@saulaski saulaski saulaski approved these changes

@AureRita AureRita AureRita approved these changes

@Julievrz Julievrz Julievrz left review comments

Assignees

@florine2623 florine2623

@AureRita AureRita

Labels
Bug fix Type: Bug fix develop Branch migration symfony migration project QA ✔️ Status: check done, code approved Wording ✔️ Status: check done, wording approved
Projects
None yet
Milestone
8.0.0
10 participants
@Progi1984 @MatShir @matthieu-rolland @atomiix @Julievrz @saulaski @AureRita @matks @florine2623 @prestonBot