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

Update the wp-comments package to use the WP REST API directly #542

Merged
merged 58 commits into from Sep 14, 2020

Conversation

michalczaplinski
Copy link
Member

@michalczaplinski michalczaplinski commented Aug 21, 2020

What:

Update the implementation of the wp-comments package to use the REST API directly.

Why:

The previous implementation has a problem with following redirects as outlined in the Feature Discussion: https://community.frontity.org/t/wordpress-comments-package/1267/53?u=mmczaplinski

How:

  • Updating thewp-comments package to use the REST API directly
  • Add the data returned by the REST API when POSTing a new comment directly to state.source.data and state.source.comment. This is handled inside of the submit() action.
  • Store the validation errors returned from WordPress in the state. The REST API returns the validation errors for each of the fields in the comment. E.g. if you send a request with a malformed author_email
  • Add a new type of data to @frontity/source
  • Removing the query, route & link from the non-URL entities that are fetched from the REST API.

Also additionally:

  • Add a new load-db.js script for local testing using the e2e testing system.
  • It also includes some improvements to the e2e testing system (adding retries & migrating the plugins file and the wp-comments e2e test file to typescript)

Tasks:

Unrelated Tasks

  • TypeScript tests
  • Update starter themes

@michalczaplinski michalczaplinski self-assigned this Aug 21, 2020
@changeset-bot
Copy link

changeset-bot bot commented Aug 21, 2020

🦋 Changeset is good to go

Latest commit: 557b2f5

We got this.

This PR includes changesets to release 3 packages
Name Type
@frontity/source Minor
@frontity/wp-comments Patch
@frontity/wp-source Patch

Not sure what this means? Click here to learn what changesets are.

Click here if you're a maintainer who wants to add another changeset to this PR

@michalczaplinski michalczaplinski changed the title Update the implementation of the E2E tests Update the wp-comments package to use the WP REST API directly Aug 21, 2020
Base automatically changed from docker-e2e-test to dev August 24, 2020 15:08
@michalczaplinski
Copy link
Member Author

michalczaplinski commented Sep 10, 2020

We are receiving same error on this PR as reported by @luisherranz previously in bradennapier/eslint-plus-action#45
Failed to generate URL to download artifact 😕

@michalczaplinski michalczaplinski marked this pull request as ready for review September 10, 2020 01:42
@michalczaplinski
Copy link
Member Author

The e2e tests are failing on the previous version of WP - I will look into this, but in the meantime you can already review it @DAreRodz :)

"taskTimeout": 60000
"taskTimeout": 60000,
"retries": {
"runMode": 3,
Copy link
Member Author

Choose a reason for hiding this comment

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

@michalczaplinski
Copy link
Member Author

The e2e tests are failing on the previous version of WP - I will look into this, but in the meantime you can already review it @DAreRodz :)

The test failures were really weird and had multiple causes:

  1. In WordPress 5.4 & 5.3, the default comment with ID == 1 that is present in the database (The one that starts with This is a test. If you want to get started with moderation... is not being returned from the REST API. You can fetch it using GET post/1/comments/1, but it's missing from the list of all comments in GET post/1/comments/! I

    I'm not quit sure why that is the case, but I've just worked around it by deleting the default comment and just adding my own to the database.

  2. Another problem seems to be that the approved / hold status of the comments depends on some factors that we have not anticipated. In the e2e tests, when I submit a test comment, the same comment appeared to be approved in 5.4 but is hold in 5.5 !

    I have first tried to work around it by checking the WP version and basically check that: wpVersion < 5.5 ? "approved" : "hold". However, with some more testing it turns out that sometimes the same comment also gets stuck on hold in 5.4.
    My only conclusion is that the approve/hold status of the comments depends on some other factors that are hard to control like time since last posted a comment from a particular IP or that the classification is non-deterministic.

Copy link
Member

@DAreRodz DAreRodz left a comment

Choose a reason for hiding this comment

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

It looks great, @michalczaplinski! 👌

.changeset/plenty-ligers-walk.md Outdated Show resolved Hide resolved
e2e/integration/wordpress-01/wp-comments.spec.ts Outdated Show resolved Hide resolved
packages/wp-comments/src/index.ts Outdated Show resolved Hide resolved
@michalczaplinski
Copy link
Member Author

michalczaplinski commented Sep 14, 2020

I will merge this now, despite the eslint failure as I have no eslint issues locally so it was most likely just a problem with the github action.

Note that I have added the /* eslint-disable */ to silence the warnings as Luis has already implemented the TSDocs in another PR

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.

Add action to create new comments [13pt]
2 participants