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

Simplify brand and brand address forms #22474

Merged
merged 14 commits into from
Jan 8, 2021
Merged

Simplify brand and brand address forms #22474

merged 14 commits into from
Jan 8, 2021

Conversation

JevgenijVisockij
Copy link
Contributor

@JevgenijVisockij JevgenijVisockij commented Dec 17, 2020 •
edited
Loading

Questions Answers
Branch? develop
Description? Simplifies product preferences form
Type? refacto
Category? BO
BC breaks? yes
Deprecations? no
Fixed ticket? Part of #16482
How to test? Simplifying Sell -> Catalog -> Brands and Brands addresses formsd. Everything must look the same aside from help now appearing under inputs instead as blue box, also some fields now will appear as required because they always were.

📓 BC Break
Backwards compatibility break introduced due to extension of TranslationAwareType and changing of some construct params by ManufacturerAddressType. This means if any module extends this type they will get an exception upon upgrading to PS version containing changes in this PR.

This change is Reviewable

@JevgenijVisockij JevgenijVisockij requested a review from a team as a code owner December 17, 2020 15:14
@prestonBot prestonBot added develop Branch Refactoring Type: Refactoring Waiting for wording Status: action required, waiting for wording labels Dec 17, 2020
@JevgenijVisockij JevgenijVisockij changed the title [WIP] Simplify brand and supplier forms [WIP] Simplify brand forms Dec 17, 2020
@JevgenijVisockij JevgenijVisockij changed the title [WIP] Simplify brand forms [WIP] Simplify brand and brand address forms Dec 17, 2020
@matks matks added the migration symfony migration project label Dec 18, 2020
@matks matks mentioned this pull request Dec 18, 2020
Open
matks
matks previously approved these changes Dec 18, 2020
View reviewed changes
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.

Code seems OK. I guess the form_row cannot be avoided.

@Julievrz Julievrz added the Waiting for author Status: action required, waiting for author feedback label Dec 23, 2020
@JevgenijVisockij JevgenijVisockij force-pushed the feature/simplify-brand-and-supplier-forms branch from fc4d219 to 08f0dac Compare December 23, 2020 14:01
@JevgenijVisockij
Copy link
Contributor Author

Hey @Julievrz thanks for the review, I implemented the wording changes.

@JevgenijVisockij JevgenijVisockij changed the title [WIP] Simplify brand and brand address forms Simplify brand and brand address forms Dec 23, 2020
@Julievrz Julievrz added Wording ✔️ Status: check done, wording approved and removed Waiting for wording Status: action required, waiting for wording labels Dec 24, 2020
@Julievrz
Copy link
Contributor

Hey @Julievrz thanks for the review, I implemented the wording changes.

Thank you very much!

@JevgenijVisockij JevgenijVisockij changed the title Simplify brand and brand address forms [WIP] Simplify brand and brand address forms Jan 5, 2021
@prestonBot prestonBot added Waiting for wording Status: action required, waiting for wording and removed Wording ✔️ Status: check done, wording approved labels Jan 5, 2021
@JevgenijVisockij JevgenijVisockij changed the title [WIP] Simplify brand and brand address forms Simplify brand and brand address forms Jan 5, 2021
@JevgenijVisockij JevgenijVisockij force-pushed the feature/simplify-brand-and-supplier-forms branch from 7c215a6 to 83ff16b Compare January 5, 2021 09:17
Progi1984
Progi1984 reviewed Jan 5, 2021
View reviewed changes
Copy link
Member

@Progi1984 Progi1984 left a comment

Choose a reason for hiding this comment

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

A small feedback from @atomiix and it's OK for me

@JevgenijVisockij @atomiix
Update src/PrestaShopBundle/Form/Admin/Sell/Manufacturer/Manufacturer…
357345d 
…Type.php


Removed uneeded comment

Co-authored-by: atomiix <tbaccelli@gmail.com>
@JevgenijVisockij
Copy link
Contributor Author

A small feedback from @atomiix and it's OK for me

It's fixed

@Julievrz Julievrz added Wording ✔️ Status: check done, wording approved and removed Waiting for wording Status: action required, waiting for wording labels Jan 6, 2021
@matks matks added Waiting for QA Status: action required, waiting for test feedback and removed Waiting for author Status: action required, waiting for author feedback labels Jan 6, 2021
@khouloudbelguith khouloudbelguith self-assigned this Jan 7, 2021
@khouloudbelguith
Copy link
Contributor

Hi @JevgenijVisockij,

It is ok ✔️
https://drive.google.com/file/d/1zH3_i-3CSOvjvxYJ6wtKJ-LKj1xgho-L/view?usp=sharing

Thank you!

@khouloudbelguith khouloudbelguith added QA ✔️ Status: check done, code approved and removed Waiting for QA Status: action required, waiting for test feedback labels Jan 8, 2021
@prestonBot
Copy link
Collaborator

QA approved, well done! Message to the maintainers: do not forget to milestone it before the merge.

@khouloudbelguith khouloudbelguith added this to the 1.7.8.0 milestone Jan 8, 2021
@khouloudbelguith khouloudbelguith mentioned this pull request Jan 8, 2021
Closed
@Progi1984 Progi1984 merged commit f18e309 into PrestaShop:develop Jan 8, 2021
@Progi1984
Copy link
Member

Thanks @JevgenijVisockij & @khouloudbelguith

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

@atomiix atomiix atomiix left review comments

@Julievrz Julievrz Julievrz left review comments

@Progi1984 Progi1984 Progi1984 approved these changes

@matks matks matks approved these changes

Assignees

@khouloudbelguith khouloudbelguith

Labels
develop Branch migration symfony migration project QA ✔️ Status: check done, code approved Refactoring Type: Refactoring Wording ✔️ Status: check done, wording approved
Projects
None yet
Milestone
1.7.8.0
Development

Successfully merging this pull request may close these issues.
7 participants
@JevgenijVisockij @Julievrz @khouloudbelguith @prestonBot @Progi1984 @atomiix @matks