Invalidate a Note after un-approving a notification

[ctarda]

Investigations for #612

Following the suggestions in this slack thread, I am invalidating the cached Note and triggering a reload after a review / comment is marked as unapproved

However, I can see how the payload returned by the API when I refresh the notification is the same as it was before marking the notification as unapproved.

It seems like a call to "sites/{site_id}/comments/{comment_id}" is not triggering a change in what gets returned when requesting to load the corresponding note again.

I don't even know if that is expected, to begin with, but I figure it is.

Anyway, I have left a bunch of print statements here, to make sure I can debug the full sequence of what is happening after marking a review as unnapproved.

Changes

• Added a method invalidateNotificationCache to NotificationDetailsViewController that dispatches a new action to invalidate the Note that was just marked as unapproved and then refresh it by dispatching the action to synchronise that note again
• Added a bunch of prints to the sequence, so that the actual payload returned by NotificationsRemote.loadNotes is dumped to the console. Notice how the payload still contains approve-comment: true

Update release notes:

• If there are user facing changes, I have added an item to RELEASE-NOTES.txt.

[mindgraffiti]

I spent an hour looking into this and here is what I know:

• a request to unapprove the comment is sent out
• a success response is received, the status of the comment is correct
• the note hash is invalidated in core data
• a new sync notes request is sent
• the SAME note ID and note Hash are returned in the API response (should the hash change? I'm not sure what to expect if everything works properly)

Is the Note Hash supposed to get sent to the API to be invalidated? I didn't see that happen anywhere.

[ctarda]

The hash is supposed to be invalidated locally in NotificationDetailsViewController. invalidateNotificationCache. If I understood the setup correctly, that should trigger a re-sync for that particular notification when we request a sync of all notifications.

Now I am a bit confused. Do I need to tell the API that we want it to "rebuild" the notification? I would expect that to happen on their end automatically when we change the approval status, but I don't know if that assumption is correct

[jleandroperez]

@ctarda (Stepping over here, catching up with my GH notifications). Just for the record:

• Exactly as you said. Invalidating the Hash should cause the Sync call to redownload the Notification
• The Comment Moderation OP should be rebuilding the notification, backend side

As we spoke over Slack, this might be caused by a backend glitch (maybe a missing call somewhere, so that the Note is regenerated).

Writing this down, here, for record purposes. Please let me know if anything!!

[houndci-bot]

Trailing Whitespace Violation: Lines should not have trailing whitespace. (trailing_whitespace)

[mindgraffiti]

@ctarda did the backend issue get reported? Can you link to the bug report here? I would like to have this PR merged in if those two things are done, that way it's "instantly" fixed when the backend bug merges in.

[ctarda]

@mindgraffiti There has been recent progress made by the friendly backend folks, as mentioned in p99K0U-1mi-p2 (comment 3175). Will update this and the original issue when there is more information available

[ctarda]

This is the PR that fixes the issue on the backend:
Automattic/jetpack#13079

I will close our PR when the jetpack fix is deployed

[ctarda]

Closing now, as there is a scheduled date for a jetpack release containing the fix:

Automattic/jetpack#13079 (comment)