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

adds synonyms moderation front end #2538

Open
wants to merge 12 commits into
base: main
Choose a base branch
from

Conversation

Courey
Copy link
Contributor

@Courey Courey commented Aug 14, 2024

Description

This is the front end for the moderation portion of synonyms (not the circular index grouped by synonyms, but the ability to create, put, delete the synonyms themselves). It is behind both a feature flag and a moderator check. The majority of the code is behind a feature flag and moderation gate, but I did create a component for pagination to reduce redundancy so I could reuse it for this work. In order to do that I had to update the circulars index page as well. That portion is NOT behind a feature flag.

Related Issue(s)

Resolves #2116

Testing

This is the moderation index with pagination. Links for each group are to the detail page
Screenshot 2024-08-13 at 4 34 31 PM

This is the detail page for a synonym group. The button to save is disabled because no additional eventIds have been added. It is enabled when a new eventId is added.
Screenshot 2024-08-14 at 10 04 06 AM

This is the error shown if you enter an eventId that doesn't exist.
Screenshot 2024-08-14 at 2 50 48 PM

This is the create view with no eventIds added.
Screenshot 2024-08-14 at 10 05 12 AM

This is the same view with eventIds added.
Screenshot 2024-08-14 at 10 05 44 AM

This is the error shown when you try to create a synonym group with an invalid eventId
Screenshot 2024-08-14 at 2 37 14 PM

@jracusin
Copy link
Contributor

When you delete synonyms, does it double check you meant to delete them? If not, it would be good to do so.

@Courey
Copy link
Contributor Author

Courey commented Aug 14, 2024

When you delete synonyms, does it double check you meant to delete them? If not, it would be good to do so.

It does not currently, but that is definitely something I can add in!

@Courey
Copy link
Contributor Author

Courey commented Aug 16, 2024

Here are updated images to show the additional request for a modal making sure you want to delete a synonym group.
I also updated the trashcan icon next to each eventId so that it says remove instead of just uses an icon.
Screenshot 2024-08-16 at 11 41 34 AM
Screenshot 2024-08-16 at 11 41 56 AM
Screenshot 2024-08-16 at 11 44 38 AM
Screenshot 2024-08-16 at 11 44 56 AM

Copy link

codecov bot commented Aug 16, 2024

Codecov Report

Attention: Patch coverage is 11.93182% with 155 lines in your changes missing coverage. Please review.

Project coverage is 6.23%. Comparing base (54ea8ce) to head (8a67744).

Files with missing lines Patch % Lines
app/routes/synonyms.new.tsx 0.00% 57 Missing ⚠️
app/routes/synonyms.$synonymId.tsx 0.00% 45 Missing ⚠️
app/routes/synonyms._index.tsx 0.00% 30 Missing ⚠️
app/routes/synonyms/synonyms.server.ts 63.63% 12 Missing ⚠️
...omponents/pagination/PaginationSelectionFooter.tsx 0.00% 8 Missing ⚠️
app/routes/circulars._archive._index/route.tsx 0.00% 2 Missing ⚠️
app/components/pagination/GCNPagination.tsx 0.00% 1 Missing ⚠️
Additional details and impacted files
@@           Coverage Diff            @@
##            main   #2538      +/-   ##
========================================
+ Coverage   6.08%   6.23%   +0.15%     
========================================
  Files        164     167       +3     
  Lines       4077    4216     +139     
  Branches     450     466      +16     
========================================
+ Hits         248     263      +15     
- Misses      3827    3951     +124     
  Partials       2       2              

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

@Courey Courey marked this pull request as ready for review August 16, 2024 15:53
app/routes/synonyms._index.tsx Outdated Show resolved Hide resolved
app/routes/synonyms._index.tsx Outdated Show resolved Hide resolved
app/routes/synonyms._index.tsx Outdated Show resolved Hide resolved
app/routes/synonyms.new.tsx Outdated Show resolved Hide resolved
app/routes/synonyms.new.tsx Outdated Show resolved Hide resolved
app/routes/synonyms/route.tsx Outdated Show resolved Hide resolved
app/routes/synonyms/route.tsx Outdated Show resolved Hide resolved
app/routes/synonyms/synonyms.lib.ts Outdated Show resolved Hide resolved
@Courey Courey requested a review from dakota002 August 28, 2024 14:28
Copy link
Member

@lpsinger lpsinger left a comment

Choose a reason for hiding this comment

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

I don't see anything in the action functions that checks that the user is authorized.

app/routes/synonyms.new.tsx Show resolved Hide resolved
app/routes/synonyms._index.tsx Show resolved Hide resolved
@@ -40,10 +33,10 @@ import {
putVersion,
search,
} from '../circulars/circulars.server'
import CircularPagination from './CircularPagination'
Copy link
Member

Choose a reason for hiding this comment

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

This module is unused now, isn't it? Please refactor to eliminate dead code.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I'm not sure what you mean here? It is used. I moved it into components>pagination>GCNPagination.tsx and it is used in the PaginationFooter which is also moved to that same directory.

app/routes/synonyms/route.tsx Outdated Show resolved Hide resolved
app/routes/synonyms.$synonymId.tsx Outdated Show resolved Hide resolved
app/routes/synonyms/synonyms.server.ts Outdated Show resolved Hide resolved
app/routes/synonyms/synonyms.server.ts Outdated Show resolved Hide resolved
@Courey
Copy link
Contributor Author

Courey commented Sep 13, 2024

With nothing on the page yet
Screenshot 2024-09-13 at 1 09 56 PM

when you type 3 or more characters
Screenshot 2024-09-13 at 1 10 33 PM

When you hover over one of the options
Screenshot 2024-09-13 at 1 10 43 PM

@Courey Courey force-pushed the courey/synonyms_index_frontend branch from 3149fdc to 8a67744 Compare September 20, 2024 13:11
Copy link
Member

@lpsinger lpsinger left a comment

Choose a reason for hiding this comment

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

Sorry this took me so long to get to. There are some merge conflicts. Would you please rebase from main?

Squashing your existing commits before rebasing might make it easier.

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.

synonym moderation UI
4 participants