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

In case multiple payment options are available #25069

Merged
merged 1 commit into from
Nov 19, 2021
Merged

In case multiple payment options are available #25069

merged 1 commit into from
Nov 19, 2021

Conversation

moncef-essid
Copy link
Contributor

@moncef-essid moncef-essid commented Jun 21, 2021

In case multiple payment options are available

Questions Answers
Branch? develop
Description? In case multiple payment options are available, the page will contain duplicate payment-form ID. Modify the ID name to make it different.
Type? bug fix
Category? FO
BC breaks? yes
Deprecations?
Fixed ticket?
How to test? Use multiple payment options, open FO checkout process, verify page contains multiple payment-form IDs
Possible impacts? If a module relies on this ID, it could not work anymore

BC Break

Change of HTML ID payment-form to payment-{$option.id}-form


This change is Reviewable

In case multiple payment options are available
@moncef-essid moncef-essid requested a review from a team as a code owner June 21, 2021 21:19
@prestonBot
Copy link
Collaborator

Hi, thanks for this contribution!

I found some issues with the Pull Request description:

  • The category should be one of these: BO, CO, FO, IN, LO, ME, PM, TE or WS (Read explanation)

Would you mind having a look at it? This will help us understand how interesting your contribution is, thank you very much!

(Note: this is an automated message, but answering it will reach a real human)

@prestonBot prestonBot added the Bug fix Type: Bug fix label Jun 21, 2021
Copy link
Contributor

@matthieu-rolland matthieu-rolland left a comment

Choose a reason for hiding this comment

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

Thank you @moncef-essid for your contribution 👍

While your modification makes sense we ask our contributors to create an issue and then properly link it in the PR description so that our PM team can properly track the project, and our QA team can understand how to validate your work:

https://github.com/PrestaShop/PrestaShop/issues/new/choose

@prestonBot prestonBot added develop Branch BC break Type: Introduces a backwards-incompatible break labels Aug 4, 2021
Copy link
Contributor

@matks matks left a comment

Choose a reason for hiding this comment

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

develop will become 8.0.0 and BC breaks are acceptable.

I approve the PR. I also updated the PR body.

@Progi1984
Copy link
Member

@moncef-essid Thanks for your contribution. Could you create an issue, please ?

@Progi1984 Progi1984 added the Waiting for author Status: action required, waiting for author feedback label Aug 5, 2021
Copy link

@NeOMakinG NeOMakinG left a comment

Choose a reason for hiding this comment

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

I agree, but I hope we don't break anything else by just changing this ID, overwise we would need to fix it in this PR

@PierreRambaud PierreRambaud removed the Waiting for author Status: action required, waiting for author feedback label Nov 8, 2021
@PierreRambaud
Copy link
Contributor

I close and reopen the PR.
We will merge it without QA.

@atomiix
Copy link
Contributor

atomiix commented Nov 18, 2021

@PierreRambaud ready to merge I guess?

@PierreRambaud PierreRambaud merged commit 0da6d3b into PrestaShop:develop Nov 19, 2021
@prestonBot
Copy link
Collaborator

Contribution merged, congratulations!

Would you mind answering our quick 1-minute survey? We would love to hear about your experience so far, it will help us improve our process for the community involved, like you. ;-)

@PierreRambaud
Copy link
Contributor

Thank you @moncef-essid

@PierreRambaud PierreRambaud added this to the 8.0.0 milestone Nov 19, 2021
@matks matks added the Needs documentation Needs an update of the developer documentation label Jul 19, 2022
@kpodemski kpodemski added Documentation ✔️ Developer documentation is up-to-date and removed Needs documentation Needs an update of the developer documentation labels Jul 28, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
BC break Type: Introduces a backwards-incompatible break Bug fix Type: Bug fix develop Branch Documentation ✔️ Developer documentation is up-to-date
Projects
None yet
Development

Successfully merging this pull request may close these issues.

9 participants