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

fix/35: replace setDescription with setDescriptionHtml #47

Merged
merged 2 commits into from
Jan 31, 2024
Merged

Conversation

Sidsector9
Copy link
Member

All Submissions:

  • Does your code follow the WooCommerce Sniffs variant of WordPress coding standards?
  • Have you written new tests for your changes, as applicable?
  • Have you successfully run tests with your changes locally?
  • Will this change require new documentation or changes to existing documentation?

Changes proposed in this Pull Request:

Closes #35 .

Steps to test the changes in this Pull Request:

  1. Add description in Woo product with more than 4096 characters and sync the product
  2. Verify that description is visible in the response from the API explorer. Use the list-catalog endpoint.

Changelog entry

Fix - Issue with syncing products with description more than 4096 characters.

@Sidsector9 Sidsector9 self-assigned this Jan 9, 2024
@jeffpaul jeffpaul added this to the Future Release milestone Jan 16, 2024
@qasumitbagthariya qasumitbagthariya added the status: blocked The issue is blocked from progressing, waiting for another piece of work to be done. label Jan 17, 2024
@qasumitbagthariya
Copy link
Contributor

qasumitbagthariya commented Jan 17, 2024

QA Update ❌


In the sandbox dashboard, the problem remains unresolved since the description field continues to be trimmed down to 4096 characters.

As discussed with @Sidsector9 this is the Square dashboard issue and we have requested the reopen the issue square/square-php-sdk#127 (comment)

Video:

Screenshare.-.2024-01-17.1_29_04.PM.mp4

Copy link
Contributor

@qasumitbagthariya qasumitbagthariya left a comment

Choose a reason for hiding this comment

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

This PR is on hold until the issue is fixed from the square.

@qasumitbagthariya
Copy link
Contributor

QA Update ✅


I have checked this issue in the fix/35 branch and the related issue is resolved and working as expected.

square/square-php-sdk#127 (comment) - Resolved

Testing Environment

  • WordPress: 6.4.2
  • Theme: Storefront 4.5.3
  • WooCommerce - 8.5.1
  • PHP: 8.0.21
  • Web Server: Nginx 1.20.2
  • Browser: Chrome
  • OS: macOS Ventura 13.3

Steps to Test- As mentioned in the PR description.
Test Results - It is working as expected.
Functional Demo / Screencast -
Special Notes - Ready for code review (Woo)

Screenshare.-.2024-01-23.8_33_35.PM.mp4

Status update:
@ankitguptaindia

@qasumitbagthariya qasumitbagthariya requested review from a team and a-danae and removed request for a team January 23, 2024 15:06
Copy link

@a-danae a-danae left a comment

Choose a reason for hiding this comment

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

Things look good to me 👍

✅ I added a description with a length of 5000 characters. The sync was successful in this branch, while it failed in trunk with a [VALUE_TOO_LONG] Field must not be greater than 4096 length error message.

Thanks for the changes here, @Sidsector9 !

@dkotter dkotter removed the status: blocked The issue is blocked from progressing, waiting for another piece of work to be done. label Jan 30, 2024
@qasumitbagthariya
Copy link
Contributor

Regression / Smoke Test Report ✅

Tested with Archive File created via "php woorelease.phar build repo_URL" (Composer version 2.5.5, npm version 8.19.4, node version 16.20.0)

Testing Environment

  • WordPress: 6.4.2
  • Theme: Storefront 4.5.4
  • WooCommerce - 8.5.2
  • PHP: 8.0.30
  • Web Server: Nginx 1.20.2
  • Browser: Chrome
  • OS: macOS Ventura 13.3
  • Git Branch: smoke-testing

Status- Working expected with Plugin Archive/Zip file same as fix specific branch.

Next Step- Ready to Merge 🚀

Status update
@ankitguptaindia @vikrampm1

@vikrampm1 vikrampm1 modified the milestones: Future Release, 4.5.0 Jan 31, 2024
@vikrampm1 vikrampm1 merged commit 58c7938 into trunk Jan 31, 2024
5 checks passed
@vikrampm1 vikrampm1 deleted the fix/35 branch January 31, 2024 14:58
@vikrampm1 vikrampm1 mentioned this pull request Jan 31, 2024
16 tasks
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Products with long descriptions causing VALUE_TOO_LONG errors when attempting to sync
6 participants