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

Change limit of filed value on ps_customized_data so that it can accepts more than 255 characters #31109

Merged
merged 1 commit into from
Feb 2, 2023
Merged

Change limit of filed value on ps_customized_data so that it can accepts more than 255 characters #31109

merged 1 commit into from
Feb 2, 2023

Conversation

lartist
Copy link
Contributor

@lartist lartist commented Jan 30, 2023 •
edited by kpodemski
Loading

Questions Answers
Branch? develop
Description? Change limit of filed value on ps_customized_data so that it can accepts more than 255 characters
Type? new feature
Category? BO
BC breaks? no
Deprecations? no
How to test? See below
Fixed ticket? Fixes #27579
Related PRs PrestaShop/autoupgrade#556
Sponsor company

This PR comes with PrestaShop/classic-theme#100

  1. create a zip from https://github.com/lartist/classic-theme/tree/change-limit-on-customized-product-value (or https://github.com/PrestaShop/classic-theme develop if Change limit of filed value on ps_customized_data so that it can accepts max 1024 characters classic-theme#100 is merged
  2. Upload the zip in theme page
  3. See the helper text below custom field value in front product page
  4. Try to put more than 255 characters. After submitting the form, nothing is truncated

Alternative testing:
You can edit manually the file themes/classic/templates/catalog/_partials/product-customization.tpl and change the maxlength property to 1024

@lartist lartist requested a review from a team as a code owner January 30, 2023 16:25
@prestonBot prestonBot added develop Branch Feature Type: New Feature labels Jan 30, 2023
mflasquin
mflasquin previously approved these changes Jan 30, 2023
View reviewed changes
FabienPapet
FabienPapet previously approved these changes Jan 30, 2023
View reviewed changes
jolelievre
jolelievre previously approved these changes Jan 31, 2023
View reviewed changes
Copy link
Contributor

@jolelievre jolelievre left a comment

Choose a reason for hiding this comment

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

Thanks @lartist

@jolelievre
Copy link
Contributor

Since your PR also depends on an update of the classic theme I think we need to explain how to use the proper branch to fully test this feature, by manually modifying the composer.json file.

@Progi1984
Copy link
Member

It's a BC : https://github.com/PrestaShop/ADR/blob/master/0017-backward-compatibility-promise.md#database-structure

image

@lartist lartist added the Waiting for QA Status: action required, waiting for test feedback label Jan 31, 2023
@lartist lartist dismissed stale reviews from jolelievre, FabienPapet, and mflasquin via cb3e2f1 January 31, 2023 14:25
jolelievre
jolelievre previously approved these changes Jan 31, 2023
View reviewed changes
@jolelievre
Copy link
Contributor

@Progi1984 yep the PR was modified to keep the same type for the column, it's only been extended/increased so it's not a BC break anymore as it's the same as changing the column constraint which is accepted in the backward policy

@jolelievre jolelievre removed the Waiting for QA Status: action required, waiting for test feedback label Jan 31, 2023
zuk3975
zuk3975 previously approved these changes Jan 31, 2023
View reviewed changes
@Progi1984
Copy link
Member

@jolelievre Totally agree ;)

nicosomb
nicosomb reviewed Jan 31, 2023
View reviewed changes
Copy link
Contributor

@nicosomb nicosomb left a comment

Choose a reason for hiding this comment

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

Do we need to update autoupgrade module?

@zuk3975 zuk3975 added the Waiting for QA Status: action required, waiting for test feedback label Jan 31, 2023
@lartist
Copy link
Contributor Author

lartist commented Jan 31, 2023 •
edited
Loading

Do we need to update autoupgrade module?

@nicosomb PrestaShop/autoupgrade#556

djoelleuch
djoelleuch reviewed Jan 31, 2023
View reviewed changes
Copy link

@djoelleuch djoelleuch left a comment

Choose a reason for hiding this comment

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

Hello @lartist ,
we can't check your PR because there is some checks were not successful .
image

Could you please resolve the problem .

Thank you !

@djoelleuch djoelleuch added Waiting for author Status: action required, waiting for author feedback and removed Waiting for QA Status: action required, waiting for test feedback labels Jan 31, 2023
@jolelievre jolelievre added the Waiting for QA Status: action required, waiting for test feedback label Jan 31, 2023
@jolelievre jolelievre removed the Waiting for author Status: action required, waiting for author feedback label Jan 31, 2023
@jolelievre
Copy link
Contributor

Hi @djoelleuch

it was just a random bug from the CI, I restarted it but it's green on all other PHP versions no reason that the last one should fail. It's under progress here https://github.com/PrestaShop/PrestaShop/actions/runs/4055061883/jobs/6979277814

@lartist
Copy link
Contributor Author

lartist commented Jan 31, 2023 •
edited
Loading

Hello @lartist , we can't check your PR because there is some checks were not successful . image

Could you please resolve the problem .

Thank you !

hi @djoelleuch I rerun it and everything is green (sometimes random failures occur.

nicosomb
nicosomb previously approved these changes Jan 31, 2023
View reviewed changes
@lartist lartist changed the base branch from develop to 8.1.x February 1, 2023 09:27
@lartist lartist dismissed stale reviews from nicosomb, zuk3975, and jolelievre February 1, 2023 09:27

The base branch was changed.

@lartist lartist changed the base branch from 8.1.x to develop February 1, 2023 09:33
@paulnoelcholot
Copy link

Hello @lartist,

I have tested your PR and I found some bugs :

On FO product page :
image

On FO checkout page :
image

On BO order :
image
if the text has spaces

On BO order :
image
if the text hasn't spaces

@paulnoelcholot paulnoelcholot 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 1, 2023
@paulnoelcholot
Copy link

Hello @lartist,

My apologies, after a second test, the problem was related to my environment.
Everything is ok for me 🎉

Thanks!

@paulnoelcholot paulnoelcholot added QA ✔️ Status: check done, code approved and removed Waiting for author Status: action required, waiting for author feedback labels Feb 2, 2023
@prestonBot
Copy link
Collaborator

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

@prestashop-issue-bot
Copy link

QA OK without required approvals !? :trollface:

jolelievre
jolelievre requested changes Feb 2, 2023
View reviewed changes
Copy link
Contributor

@jolelievre jolelievre left a comment

Choose a reason for hiding this comment

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

Hi @lartist

can you please rebase this one based on 8.1.x and change the target of the PR?

Thanks

@Progi1984 Progi1984 added this to the 9.0.0 milestone Feb 2, 2023
jolelievre
jolelievre approved these changes Feb 2, 2023
View reviewed changes
Copy link
Contributor

@jolelievre jolelievre left a comment

Choose a reason for hiding this comment

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

Thanks @lartist

This will be a new feature for v9

@jolelievre
Copy link
Contributor

Thanks @lartist

@jolelievre jolelievre merged commit a61f5b5 into PrestaShop:develop Feb 2, 2023
This was referenced Jul 29, 2023
Closed
Merged
Closed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@djoelleuch djoelleuch djoelleuch left review comments

@paulnoelcholot paulnoelcholot paulnoelcholot approved these changes

@jolelievre jolelievre jolelievre approved these changes

@zuk3975 zuk3975 zuk3975 left review comments

@nicosomb nicosomb nicosomb left review comments

@FabienPapet FabienPapet Awaiting requested review from FabienPapet

@mflasquin mflasquin Awaiting requested review from mflasquin

Assignees
No one assigned
Labels
develop Branch Feature Type: New Feature QA ✔️ Status: check done, code approved
Projects
None yet
Milestone
9.0.0
Development

Successfully merging this pull request may close these issues.

Prestashop customization value truncated at 255 characters