WordPress comments package

No, as far as I know.

You can know if the comment was accepted but the only way to differentiate between approved or unapproved is to check the content of the URL to which you are being redirected.

And you won’t have that URL unless you follow the redirection.

One thing that comes to my mind is to update the comments after a submission with

await actions.source.fetch(..., { force: true });

and look for the comment that was published right before in state.source.comment. If it appears there we can assure it is approved. Otherwise, we could assume that it is unapproved.

I see.

If we use the redirection approach, what do we gain?

I’ll write here a little summary of the pros and cons of each approach this weekend.

Regarding CORS, I realized it could give problems for both approaches in case the response status is different than 3XX. If it returns 200 or 409 (the other two cases covered) I guess the browser will complain.

EDIT: confirmed, we cannot avoid CORS problems in those cases unless we add Access-Control headers in WordPress. I guess the only way to overcome this problem would be a server extension, but that’s not an option at this moment. :pensive:

Interesting. Well, if we cannot avoid CORS then maybe the upcoming Frontity WordPress plugin needs to be a requirement for this package. It could add CORS for the decoupled domain.

Is there any other option we can consider? The REST API is not an option right now, is it?

I’ve run some tests to understand this better and it seems the correct approach using the REST API would be to make a POST request to /wp/v2/comments as explained in the handbook. It seems the problem was that you cannot post a comment if you’re not logged in, even if the option "Users must be registered and logged in to comment " is disabled in WordPress.

I’ve seen that adding the following code allows posting new comments from the REST API if you’re not logged in:

add_filter( 'rest_allow_anonymous_comments', '__return_true' );

Once I added that line, I was able to POST new comments using that endpoint. Just passing the params we were using in the current implementation, WordPress returns a response JSON with all the info we need: the id, the url, if it’s approved or on hold…

Would this be an option to consider? Would it make sense to make a Pull Request to WordPress REST API to add this code if that option is disabled?

Btw, this approach would solve the CORS problems as it would be a common REST API request. This is the documentation of the filter used -> https://developer.wordpress.org/reference/hooks/rest_allow_anonymous_comments/

1 Like

Oh, thanks @SantosGuillamot!

I think this is a great idea and simple to implement. Also, a POST request to /wp/v2/comments/ returns the comment added to WordPress in the same format a GET requests does so we would have all the information needed to know if the comment was accepted or not, just reading the status property.

And it is just a line of code in WordPress to make it work. :raised_hands:

If we follow this idea, we would have to change the request and response logic but it wouldn’t be a difficult task.

One more thing, thinking about the @luisherranz’s idea of adding a submissions array for each post, in this case that array could be simply the array of comments received for each POST request to /wp/v2/comments, as they would already have all the needed properties (id, date and status).

Ok, that’s a much better idea. Great work @SantosGuillamot :clap:

I guess we could add that line to our plugin in the future, so the only requirement for the wp-comments package would be to have our Frontity plugin installed.

I’m glad you like Mario’s idea, @luisherranz. :blush:

I guess it won’t take long so I’ll try to implement it during this week.

Should we create a PHP plugin for this, like what we did for the Head Tags plugin?

Yes, I think it makes sense to consider it. Although for this first beta version it’s okay if users add that line into their functions.php.


I’ve been doing more tests using WordPress REST API and these are my findings so far:

  • Even after adding the PHP code mentioned above, the setting "Users must be registered and logged in to comment " keeps working as expected. This means that if you enable it from the WordPress dashboard, users won’t be able to comment if they are not logged in.

  • It seems the author_name and the author_email are not required. If you don’t pass them it just creates an "Anonymous comment. We can find the fields accepted by the comments endpoint at the REST API handbook. I think the only required fields are content and post.

  • If you pass an author id, you have to be logged in to be allowed to comment.

  • If you want to associate a comment with an author, you have to pass the author id, just with the name and the email doesn’t seem to be enough, even if you’re logged in. If you pass only the name and the email, it allows you to create the comment, but the author id field will be 0. It seems it doesn’t detect automatically the author id just with the email and name.

  • It allows you to post a comment with an incorrect parent (an id that doesn’t exist yet).

  • All the different errors have the same format, so it’d be easier to handle them. We could reuse the message WordPress is sending they are pretty descriptive. These are some examples:

Author id that doesn’t exist:

{
    "code": "rest_comment_author_invalid",
    "message": "Invalid comment author ID.",
    "data": {
        "status": 400
    }
}

Duplicate comment:

{
    "code": "comment_duplicate",
    "message": "Duplicate comment detected; it looks as though you’ve already said that!",
    "data": {
        "status": 409
    }
}

Empty comment:

{
    "code": "rest_comment_content_invalid",
    "message": "Invalid comment content.",
    "data": {
        "status": 400
    }
}

Pass a non-existing post:

{
    "code": "rest_comment_invalid_post_id",
    "message": "Sorry, you are not allowed to create this comment without a post.",
    "data": {
        "status": 403
    }
}

Post too many comments:

{
    "code": "comment_flood",
    "message": "You are posting comments too quickly. Slow down.",
    "data": {
        "status": 400
    }
}

Comments are closed for an specific post:

{
    "code": "rest_comment_closed",
    "message": "Sorry, comments are closed for this item.",
    "data": {
        "status": 403
    }
}
4 Likes

My main concern with how to move forward with this is how to test it well. I see three options:

  1. Just make the required changes and test it manually using a local instance of wordpress with the add_filter( 'rest_allow_anonymous_comments', '__return_true' ); added.
  2. Create e2e tests (using the current e2e cypress tests) that use a “remote” WP instance at test.frontity.org. This remote instance would also have to allow comments and add the rest_allow_anonymous_comments filter.
  3. Finish the new e2e testing system that uses the WP instances and write the tests using the new system.

I don’t think that 1 & 2 are very good options and I would vote for 3.

The only issue I see with 3. though is that currently the e2e tests do not support adding custom code to the WP instances. But, we could mitigate that by creating a simple 1-file frontity plugin which just includes the add_filter('rest_allow_anonymous_comments', '__return_true'); and nothing else. Then we can install this plugin in the WP instance and run tests against it.

The downside of 3. is that, of course, this is going to take longer than just 1. and 2.

@SantosGuillamot

In my tests it seems that both are indeed required :thinking:

Otherwise, it fails with:

{
  "code": "rest_comment_author_data_required",
  "message": "Creating a comment requires valid author name and email values.",
   "data": { "status": 400 }
}

Can you verify this?

@SantosGuillamot Oh, dang it, I’ve just noticed that there is the following setting in WP Dashboard:

So, if a user checks this setting, they will have to put the author_name and author_email in the request.

Yep, that’s it! I think it’s great that setting works by default :slightly_smiling_face:

This was the latest discussion we had on the on the topic:

(This is just for the record, skip to the next post for a better proposal)

In the beginning of the video, I’m talking about not storing the comment form data in the frontity state, but I thought about this some more and it’s probably not a good idea so you can forget about it for now :smiley: .

I have a better idea for improving the API (in the future), but I think we shouldn’t focus on it right now. Skip to the very end for a suggested API.


There are other small changes that I want to propose:

  1. 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, the API returns:

    { 
      "code": "rest_invalid_param",
      "message": "Invalid parameter(s): author_email",
      "data": {
        "status": 400,
        "params": {
          "author_email": "Invalid email address."
        }
      }
    }  
    

    With this info available, I think that we should store the validation errors in the state. The error message from data.params.author_email should be stored under form.errors This would look like:

    export interface Form {
      fields: Fields;
      submitted?: Submitted;
      errors: CommentErrors; // <--- store the errors here.
    }
    
    // It's kind of an awkward type, but it's easier to iterate over all the errors this way.
    // if it was an object, we would have to do `Object.entries()` first. 
    type CommentErrors = {name: keyof Fields, error: string}[]
    
  2. Add the newly submitted comment to state.source.comments directly. We can do that because the REST API returns the newly created resource after a successful POST request. This is what I get after posting a new comment.

    {
     "id": 3,
     "post": 1,
     "parent": 0,
     "author": 0,
     "author_name": "Michal",
     "author_url": "",
     "date": "2020-08-28T23:19:15",
     "date_gmt": "2020-08-28T23:19:14",
     "content": {
         "rendered": "<p>hello this is a postman test</p>\n"
     },
     "link": "http://localhost:8080/hello-world/#comment-3",
     "status": "hold",
     "type": "comment",
     "author_avatar_urls": {
         "24": "http://1.gravatar.com/avatar/160475e57835d6b02f6bfadbb05d853c?s=24&d=mm&r=g",
         "48": "http://1.gravatar.com/avatar/160475e57835d6b02f6bfadbb05d853c?s=48&d=mm&r=g",
         "96": "http://1.gravatar.com/avatar/160475e57835d6b02f6bfadbb05d853c?s=96&d=mm&r=g"
     },
     "meta": [],
     "_links": {
      //...  cut out for brevity
    

    This would also mean that we would not need to store the comment data on the submitted property. We can rename this property to status and use it to store only the submission status.

    export interface Form {
      fields: Fields;
      status?: Status;
      errors: CommentErrors;
    }
    
    export interface Status {
      isPending: boolean;
      // isOnHold: boolean;     not needed, because we have that in the Response
      // isApproved: boolean;   not needed either.
      isError: boolean;
      
     /**
      * The error message that was returned by the REST API
      */
      errorMessage: string;
      
     /**
      * The error code. Those are defined internally in the WordPress REST API.
      * @example rest_comment_invalid_post_id
      */
      errorCode: string;
       
     /**
      * The HTTP status code that might have been received 
      * from the WordPress REST API.
      */
      errorStatusCode?: number;
    
      // timestamp: number;    I think that is also not needed anymore
      // id?: number;          Not needed anymore either
    }
    

    Then, we would also have to actually populate the data in the state using populate() and insert the new comment in the existing tree of comments:

    const populated = await populate({ response, state });
    
    /* This is the part that I'm not sure about. 
       I think that the comments are sorted ascending by the `id` right? 
       So, the comments (on the same level) will always be sorted by id?
       Something like:
       
       comment 1
          comment 4
          comment 5
       comment 2
       comment 3
    */
    
    // Then we can iterate over the data.items to insert the new comment here.
    
    const { id } = await response.json();
    const data = state.source.data[route];
    
  3. Provide an updateField() action.
    I think we in addition to the updateFields() (plural) we should have an action that can update an individual field. This would have better ergonomics for the user and also result in only rerendering one field instead of the the whole form when updating a field value. It could be simply something like:

    actions: {
    	updateField: ({ state }) => (postId, name, value) => {
    		state.comments.form[postId]?.fields?[name] = value;
    	}
    }
    

Future API improvement

I was concerned about interoperability with popular form libraries like formik or react-hook-form, which are the idiomatic way to create forms in react. Rolling your own forms in react is a pain in the ass without those :sweat_smile:.

I was also thinking how to generally improve the developer experience of frontity.

We are going to provide the basic primitives for working with the WP comments which are:

  • form state,
  • submit() action
  • updateFields() action
  • updateField()` action

I propose that we also provide a useComments() hook which should cover 90% of the most basic cases which is just wiring some fields to the state, showing errors and submitting the form.

This way, we can recommend the users to use the hook for most cases and only reach to use the underlying primitives directly if the hook is not enough!

(the ref-based API is inspired by the react-hook-form )

const useComments = () => {
  const { state, actions } = useConnect();
  const { id: postId } = state.source.get(state.router.link);

  // It's a bit of a hack but it works well!
  const register = useCallback((ref) => {
    if (ref) {
      ref.addEventListener("input", ({ target: { name, value } }) => {
        actions.updateField(postId, name, value);
      });
    }
  }, []);

  const submit = (e) => {
    e.preventDefault();
    actions.comments.submit(postId);
  };

  return { submit, register, errors: state.comments.form.errors };
};

And for an example of how you’d use it:

const CommentComponent = connect(() => {
  const { register, submit, errors } = useComments();
  
  return (
    <form onSubmit={submit}>
      <input name="author_name" ref={register} />
      <input name="author_email" ref={register} />
      
      {/* We could show both client-side validation and backend errors here. */}
      {errors && errors.map(err => 
        <span>{err.name} : {err.errorMessage} </span>
      )}
      
      <input type="submit" />
    </form>
  );
})

However, I think we can work on this in the future because:

  1. It’s not necessary to use the comments.
  2. It only works if you attach the ref directly to the DOM element so wont work out of the box with most component libraries like Chakra, Bootstrap, etc. I think we would need to mess with forwardRef to make it work.
  3. There are probably many other edge cases that I didnt think of yet :smiley:

Some feedback for you, Michal! :grin:

  1. Store the validation errors returned from WordPress in the state .

    Yup, I also thought of storing that information in the state when I talked with Mario about using the REST API. My initial idea was to store the same error object received as it is in the submission status so you would access the validation errors doing something like this:

    form.status.error.data.params
    

    But if you think there is a benefit of creating a specific property in the state, that’s OK for me.

    I’m not sure about it is better for that property to be an array. How are those errors going to be consumed? Just printing an enumeration of problems for the whole form? Or an error message in each field?

  2. Add the newly submitted comment to state.source.comments directly .

    I agree with that. isOnHold and isApproved would not be needed anymore but at least we need a property to know if there was a successful request so we can print a message in React that the comment was published correctly. If not, there would not be any difference in the state if you have published comments or you have never done it.

    As I said before, I would store the same error object received from WordPress so I would change the status property to have the following structure:

    export interface Status {
      /**
      * Request has not finished yet.
      */
      isPending: boolean;
    
      /**
      * Request has finished successfully.
      */
      isSubmitted: boolean;
    
      /**
      * Request has failed.
      */
      isError: boolean;
    
      /**
      * The error object that was returned by the REST API.
      * (having WpError the same structure than the error response).
      */
      error: WpError;
    }
    

    Again, if you think it is better to not use the same structure WordPress is using for errors I have no problem with using our own schema.

    That would be really great. :blush:

  3. Provide an updateField() action .

    Actually, the current implementation of updateFields() allows you to update a single field, you just need to pass an object with that property, e.g.

    updateFields({ email: "[email protected]" });
    

    If you think it is confusing we can add the updateField() action, no problem with that. :+1:


BTW, I really like your proposal for future API improvement. :clap: