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

Remove notice on field value on webservice #18006

Merged
merged 2 commits into from Apr 21, 2020
Merged

Remove notice on field value on webservice #18006

merged 2 commits into from Apr 21, 2020

Conversation

Klemart3D
Copy link
Contributor

@Klemart3D Klemart3D commented Mar 5, 2020 •
edited

Update for #12213
To fix #12056

Questions Answers
Branch? develop
Description? Check if $field['value'] is set before display it
Type? bug fix
Category? WS
BC breaks? no
Deprecations? no
Fixed ticket? Fixes #12056
How to test? Check API with NULL values in place of empty values into database. eg: try api/customers/1 URL with a "website" or "company" value set to NULL into DB in place of empty field.

This change is Reviewable

@Klemart3D Klemart3D requested a review from a team as a code owner March 5, 2020 17:17
@prestonBot prestonBot added develop Branch Bug Type: Bug labels Mar 5, 2020
Progi1984
Progi1984 previously approved these changes Mar 10, 2020
View reviewed changes
@Progi1984 Progi1984 requested a review from a team March 10, 2020 16:26
PierreRambaud
PierreRambaud suggested changes Mar 13, 2020
View reviewed changes
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.

Do not use empty, the value can be false, 0, null

@Klemart3D
Copy link
Contributor Author

Klemart3D commented Mar 13, 2020 •
edited

Do not use empty, the value can be false, 0, null

@PierreRambaud It was your suggestion:
https://github.com/PrestaShop/PrestaShop/pull/12213/files/2e87ccdc94a1228acdf9042c843b33a36f377343#r249060236

@PierreRambaud
Copy link
Contributor

Do not use empty, the value can be false, 0, null

@PierreRambaud It was your suggestion:
https://github.com/PrestaShop/PrestaShop/pull/12213/files/2e87ccdc94a1228acdf9042c843b33a36f377343#r249060236

Lol 😅 I read it too fast last time and don't check the original behaviour. My bad :/

@PierreRambaud PierreRambaud mentioned this pull request Mar 13, 2020
Closed
@Klemart3D @PierreRambaud
Replace empty by array_key_exists
e33c966 
Co-Authored-By: GoT <PierreRambaud@users.noreply.github.com>
@Progi1984 Progi1984 added the Waiting for QA Status: action required, waiting for test feedback label Mar 13, 2020
@khouloudbelguith khouloudbelguith self-assigned this Apr 20, 2020
@khouloudbelguith
Copy link
Contributor

Hi @Klemart3D,

Your PR is OK ✔️
But, in the develop branch, I can't reproduce the issue.

Ping @PrestaShop/prestashop-core-developers can we accept this PR?

Thanks!

@khouloudbelguith khouloudbelguith added the Waiting for dev Status: action required, waiting for tech feedback label Apr 20, 2020
@PierreRambaud PierreRambaud removed the Waiting for dev Status: action required, waiting for tech feedback label Apr 21, 2020
@PierreRambaud
Copy link
Contributor

@khouloudbelguith Yes, only to be consistent with the whole method content :)

@khouloudbelguith khouloudbelguith added QA ✔️ Status: check done, code approved and removed Waiting for QA Status: action required, waiting for test feedback labels Apr 21, 2020
@khouloudbelguith khouloudbelguith added this to To be merged in PrestaShop 1.7.8.0 Apr 21, 2020
@khouloudbelguith khouloudbelguith added this to the 1.7.8.0 milestone Apr 21, 2020
@matks matks merged commit cb1cf20 into PrestaShop:develop Apr 21, 2020
@matks matks changed the title Check if $field['value'] is set before display it Remove notice on field value Apr 21, 2020
@matks matks changed the title Remove notice on field value Remove notice on field value on webservice Apr 21, 2020
@matks
Copy link
Contributor

matks commented Apr 21, 2020

Thank you @Klemart3D

@matks matks moved this from To be merged to Done in PrestaShop 1.7.8.0 Apr 21, 2020
@matks matks mentioned this pull request Apr 21, 2020
Closed
@Progi1984 Progi1984 removed this from Done in PrestaShop 1.7.8.0 Apr 21, 2020
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@PierreRambaud PierreRambaud PierreRambaud approved these changes

@Progi1984 Progi1984 Progi1984 approved these changes

@matks matks matks approved these changes

Assignees

@khouloudbelguith khouloudbelguith

Labels
Bug Type: Bug develop Branch QA ✔️ Status: check done, code approved
Projects
None yet
Milestone
1.7.8.0
Development

Successfully merging this pull request may close these issues.

[PHP Notice #8] Undefined index: value (classes/webservice/WebserviceOutputXML.php, line 128)
6 participants
@Klemart3D @PierreRambaud @khouloudbelguith @matks @Progi1984 @prestonBot