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

Feature/unreplied comments filter #14373

Merged
merged 20 commits into from Apr 2, 2021
Merged

Conversation

khaykov
Copy link
Member

@khaykov khaykov commented Mar 30, 2021

This PR adds an Unreplied comments filter:
Image from Gyazo

Here is the companion Flux-C PR.

Unreplied filter shows recent comments that are:

  • Top level (not a reply to other comment)
  • Not made by a logged in user.
  • Do not have a reply from logged in user.
  • Is Approved or Pending.

Unreplied filter has no paging, and only shows unreplied comments from 100 most recent comments.

To test:

Comment structure you are looking to test looks like this:
Image from Gyazo

  • Navigate to Comments List and select an Unreplied filter.
  • Make sure that only comments that meet following conditions are visible
    • Top Level (direct reply to post, and not to another comment).
    • Not made by logged in user.
    • Do not have a reply from logged in user.
    • Is Approved or Pending.
  • Open an Unreplied comment and reply to it.
  • Navigate back to Comments List, and confirm that the comment you replied too is no longer in Unreplied comments list.

PR submission checklist:

  • I have completed the Regression Notes.
  • I have considered adding accessibility improvements for my changes.
  • I have considered if this change warrants user-facing release notes and have added them to RELEASE-NOTES.txt if necessary.

@khaykov khaykov added this to the 17.1 milestone Mar 30, 2021
@peril-wordpress-mobile
Copy link

peril-wordpress-mobile bot commented Mar 30, 2021

You can trigger optional UI/connected tests for these changes by visiting CircleCI here.

@peril-wordpress-mobile
Copy link

peril-wordpress-mobile bot commented Mar 30, 2021

You can test the changes on this Pull Request by downloading the APK here.

…ithub.com:wordpress-mobile/WordPress-Android into feature/unreplied-comments-filter
@khaykov khaykov marked this pull request as ready for review March 30, 2021 20:15
@khaykov khaykov requested a review from develric March 30, 2021 20:15
Copy link
Contributor

@develric develric left a comment

Choose a reason for hiding this comment

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

hi @khaykov , did a first pass here and code looks good overall 👍 , also made some basic testing with .COM and self.hosted and didn't find issues; I'm going to make some more test tomorrow and setting up the scenarios you were describing in PR description, in case you want to consider them meanwhile, leaving couple of np.

  1. The empty state for the UNREPLIED tab is the generic one, maybe we could use a specific one like in the other tabs

  1. Not sure if you planned to do it separately, maybe worth to add unit testing to CommentLeveler. wdyt?

@khaykov
Copy link
Member Author

khaykov commented Apr 1, 2021

Thanks, @develric ! Implemented your suggestions 👍

Copy link
Contributor

@develric develric left a comment

Choose a reason for hiding this comment

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

Hey @khaykov 👋 ! Thanks for the changes. From the tests I've done it seems to work well for me 👍 ! Just wanted to check with you couple of things to be sure I'm not missing anything 🙇

  1. Tested with pure self hosted sites. Not sure if it's my setup but I was not able to get the account associated with an email so the comment leveler logic was not working as expected for me. What I tried was to set the email in Help and Support > Contact email but no luck. Note I was using a pure self hosted and not logged in in the app with any other WP.com account. Not sure if we can set the account email somewhere else in the app. Again, sorry in case it's obvious 🙃
  2. This one is not relevant to the PR but more as a topic to face in one of our upcoming comments enhancements projects I think. In my scenario I noticed that whereas when commenting from the web page (also from mobile browser) a submitted comment was immediately visible on the threaded comments, when commenting from the app it wasn't (one time only I actually got it but after lot of time). Wondering if we should trigger some dispatcher from the API endpoint (cc @aerych )

I think we are actually fine to go but let me know wdyt (mostly about point 1) 🙇

…ess-mobile/WordPress-Android into feature/unreplied-comments-filter
@khaykov
Copy link
Member Author

khaykov commented Apr 2, 2021

Good catch! I added a separate logic for self-hosted sites - we will try to get an email from Site object (we add it there by requesting a user profile).

Copy link
Contributor

@develric develric left a comment

Choose a reason for hiding this comment

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

Yep 👍! Was looking into the last pushes and tested it already. I think it works really well, thanks @khaykov 🙇 ! :shipit:
Feel free to merge it after fluxc is tagged 👍

@khaykov khaykov merged commit 31dbbe5 into develop Apr 2, 2021
@khaykov khaykov deleted the feature/unreplied-comments-filter branch April 2, 2021 18:54
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

2 participants