Cv2 4498 edit article #1985
Merged
Cv2 4498 edit article #1985
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Added a new RatingSelector item Added created_by date Added items needed for editing to cards Removed reliance on project_media
edit status and publish values for fact checks set description as summary for fact checks claim is now claim_description verification_statuses now sent to create form
sarahkeh
requested review from
amoedoamorim,
brianfleming and
danielevalverde
as code owners
brianfleming
requested changes
Jun 25, 2024
There was a problem hiding this comment.
Hi @sarahkeh I starting looking at this and have some things to address. I will continue to take a look, but didn't want to hold you up looking into somethings:
- The created and edited areas are not showing the user information of who created or saved, I just see the dates:
- As discussed on a call at some point, we are going to skip trash for now, so that should be removed from the footer
- There is no validation happening on required fields right now. The components should all have an ability to get the error state when appropriate
- Just like you did with the "new" button interaction, we need to close and open each article when clicked from the list, currently the slideouts will "stack", so we should match the behavior
I will keep looking, but as said, wanted to get somethings in front of you. Let me know if you have any questions or if I can clarify anything!
danielevalverde
requested changes
Jun 25, 2024
There was a problem hiding this comment.
Hi, @sarahkeh :)
I noticed that when clicking on the "Unpublished Report" button from the forms, it redirects to the wrong URL:
Could you please update the handleGoToReport function to redirect to the correct URL from the list page as well?
amoedoamorim
requested changes
Jun 27, 2024
Removed trash for now, hid the footer for editing Publish status only shows when there is an associated project media and the link is updated to go from the Articles list Removal of Created By, name is added for Last Saved: Pulled everything from ArticlesCard except onClick event to Articles file The save after editing a field now uses existing FactCheck/Claim/Explainer object + new field value, rather than just the state value, to fix a most recently updated field not saving sometimes Removed unused fields added to ArticleCard
brianfleming
reviewed
Jul 3, 2024
danielevalverde
requested changes
Jul 4, 2024
There was a problem hiding this comment.
Hi, Sarah :)
i am getting this error when clicking on an article on list view:
ArticleForm.js:40 Uncaught TypeError: Cannot read properties of null (reading 'map') at ArticleForm (ArticleForm.js:40:1)
I believe It is happening because my team doesn't have tags, it can be fixed by adding a safe navigation on this part of the code:
team.teamTags?.map https://github.com/meedan/check-web/blob/cv2-4498-edit-article/src/app/components/article/ArticleForm.js#L40
And I'm not sure how to test the edit forms. I tried editing an article and closing the form, but the changes are not being saved.
brianfleming
reviewed
Jul 9, 2024
There was a problem hiding this comment.
Couple things I wanted to check on @sarahkeh and let the others handle technical review:
- There are a lot of relay warnings that get generated when the edit slideout opens, i just wanted to flag these to make sure they are not important:
such as:Warning: RelaySelector: Expected object to contain data for fragmentArticleForm_team, got...` - Editing the Status of the Fact-Check doesn't seem to work, it never updates. Is this a bug or a know thing?
brianfleming
previously requested changes
Jul 9, 2024
There was a problem hiding this comment.
@sarahkeh something to address other than my previous questions, we should remain consistent with the slideouts and click interactions. So a couple things I noticed, and you can see them in the attached video:
- Currently when editing, I can only open 1 slideout and any other clicks do nothing. We should have it behave like the create button does, where it replaces the previous.
- Conversely, If you open an article to edit, then click the create button, it stacks instead of replacing:
Screen.Recording.2024-07-09.at.4.47.38.PM.mov
@brianfleming I think what's happening with the Status is that it went from a value tied to a media item to a value called Rating that is tied to a Fact Check. What the form does is update and save the Fact Check Rating, but I think the list displays the media item's Status, not the Rating. I thought @caiosba said these values were synced but maybe that work is pending? |
|
Yes, the form should set and read the |
@brianfleming I've just pushed the change for the top point but the second is much more complicated. I can prevent the create form opening over the edit and vice versa (by checking for the class name) but I can't close a Create form from inside the Articles List as that's a value that exists within the Create New Article component. |
There was a problem hiding this comment.
Ok, I will let the engineers approve or ask for changes. the reality is that this is getting very long lived and we need to get moving. With the creation of the rating ticket to be addressed which is currently the most confusing part for users, lets get this into the epic branch so we can easily evaluate the completness of this feature set
brianfleming
dismissed
their stale review
to let engineers do tech review and approve or ask for changes
|
Agree with @brianfleming and aligned with @amoedoamorim that he will do the tech review soon. |
|
Changes that have been added:
|
danielevalverde
pushed a commit
that referenced
this pull request
Jul 22, 2024
* Changes for editing a fact check and explainer Added a new RatingSelector item Added created_by date Added items needed for editing to cards Removed reliance on project_media * Add edit to form, added additional info in edit form edit status and publish values for fact checks set description as summary for fact checks claim is now claim_description verification_statuses now sent to create form * Added error checking for required fields Removed trash for now, hid the footer for editing Publish status only shows when there is an associated project media and the link is updated to go from the Articles list Removal of Created By, name is added for Last Saved: Pulled everything from ArticlesCard except onClick event to Articles file The save after editing a field now uses existing FactCheck/Claim/Explainer object + new field value, rather than just the state value, to fix a most recently updated field not saving sometimes Removed unused fields added to ArticleCard * remove testing console log * click on general card area, not tag buttons, added safe nav * click on article without having to close form first * Paired code review --------- Co-authored-by: Brian Fleming <[email protected]> Co-authored-by: Alexandre Amorim <[email protected]>
caiosba
added a commit
that referenced
this pull request
Jul 22, 2024
* Explainers: rail, drawer, and navigation (#1948) Add new rail item for Articles Add tooltip to new rail item Add new navigation item “Explainers” for articles Reference: CV2-4501 * Explainers list (#1953) List explainers under a new route /:team/articles/explainers. Requires Check API branch with the same name. - New component <Articles /> (and unit tests) which basically reuses other existing components - the idea is to use it for both explainers (this PR) and fact-checks (coming soon in another PR). - New component <Explainers /> that uses <Articles />. - Extracted the paginator from the feed clusters page as a standalone component <Paginator /> now used for articles and feeds. Reference: CV2-4500. * Feature/cv2-4440: articles rail drawer and navigation add claims fact checks published and imported (#1954) Refactoring: moving "Published" and "Imported" lists from Tipline rail to Articles rail Add a new Article button component Add a new ArticleCoreListCounter component update list routes Reference: CV2-4440 * Fact-checks list (#1958) List of fact-checks. Reference: CV2-4145. * Epic 4441 misc updates (#1957) * change 404 language to be less cutsey * add new beige button themes and apply to side navigation new buttons * update beige button colors * add missing classname in main button for new beige color * fix some consistency issues on the side navigation and restyle custom lists to have an easier to read layout * add missing beige prop type to main button * Refactoring/CV2-4450: remove claim and rating from add item dialog and make enhancements (#1959) * Remove Claim and Rating from Add Item dialog Remove the Claim text field from the Add item dialog Remove the rating selection from the Add item dialog Re-title Dialog and button from Add item to Add Media Rename button from 'Add Item' to 'Add Media' Reference: CV2-4450 * Added creation forms for Explainers and Fact Checks (#1960) * Added creation forms for Explainers and Fact Checks ExplainerForm handles the mutations for creating or updating an explainer article. The explainer article requires title, summary, and language. Tags and url are optional. ClaimAndFacCheckForm handles the mutations for creating or upating a fact check. A claim description is first required before removing the overlay for the Fact Check section. Form components were mostly taken from the MediaFactCheck component. An update was made to the LnaguagePicker to allow for the field to be required for the explainer. cv2-4498 and cv2-4447 * Connect forms with links on the Articles page, made necessary updates to styling and passed down team info needed for saving any new Form * Request pr changes for input forms Opening another slideout from the New Article menu will close the current slideout. Menu will close when a selection is made. Flash message now uses the useContext hook Changed error handling failed saves, will use the API error and fallback on a generic message Specified the possible strings for articleType and mode values Removed the reliance on values for TranslatedMessages which was preventing the messages from truly being translated Removed error value that was left over from testing * removed console statement * Added team tags for the forms, made additional fixes to simplify value assignments. Added tag_texts to Projects query since the New Article button lives within that object * saving the wrong info * List workspace tags on article card tag picker. (#1974) Reference: CV2-4734. * CV2-4665: filter by report information * CV2-4665: declare propTypes for statuses * Add articles to item from item page (#1983) See Figma designs in referenced Jira ticket for details. But the main ideas here are: - Delete the left sidebar from the item page (the one with claim and fact-check) - Add a new right sidebar called "Articles" - In that sidebar, for items with no articles (so, no explainers and no fact-check), show a button to create new article and list articles to be applied to item - Clicking on an article (explainer or fact-check) adds it to the current item - The articles in that right sidebar have a hover style - When the item has an article, the right sidebar shows only the "New article" button - this case is going to be handled by another ticket Reference: CV2-4626 * Run [full ci] * fix merge typo/linter issue * List styles more consistent (#1989) * clean up some inconsistencies between lists and use more shared styles while reviewing epic progress [CV2-4441] * remove unnecessary toolbar component [CV2-4441] * remove unnecessary articles specific list styles [CV2-4441] * clean up some differences between shared feed headers [CV2-4441] * remove unnecessary import of classnames * user shared css divider styles instead of material [CV2-4441] * quiet down browser console error * remove unnecessary fieldset title, now that claim has been removed * run tests for this branch * Fixing bad merge * [full ci] * Feature/4628 search for articles to add (#1990) * Implement search existing article button and slideout * add unit test * add BEM class * Add empty state for article search --------- Co-authored-by: Brian Fleming <[email protected]> * Fixes 3 bugs for the articles lists pages (#1997) * Ticket CV2-4886: Adding titles to articles list pages * Fixing unit test * Ticket CV2-4889: Display number of articles on Articles tab * Ticket CV2-4888: Move sorting on Articles lists to be inline with filters * Fixing unit test * Cv2 4498 edit article (#1985) * Changes for editing a fact check and explainer Added a new RatingSelector item Added created_by date Added items needed for editing to cards Removed reliance on project_media * Add edit to form, added additional info in edit form edit status and publish values for fact checks set description as summary for fact checks claim is now claim_description verification_statuses now sent to create form * Added error checking for required fields Removed trash for now, hid the footer for editing Publish status only shows when there is an associated project media and the link is updated to go from the Articles list Removal of Created By, name is added for Last Saved: Pulled everything from ArticlesCard except onClick event to Articles file The save after editing a field now uses existing FactCheck/Claim/Explainer object + new field value, rather than just the state value, to fix a most recently updated field not saving sometimes Removed unused fields added to ArticleCard * remove testing console log * click on general card area, not tag buttons, added safe nav * click on article without having to close form first * Paired code review --------- Co-authored-by: Brian Fleming <[email protected]> Co-authored-by: Alexandre Amorim <[email protected]> * Fixing the build (probably a bad merge) * Card Improvements [CV2-4887] (#1996) * start to clean up sandbox for cluster card. Allow a fact-check link without a description in cluster cards * add optional link and tags to article card in sandbox * add published to sandbox article card and improve unread visuals * hide language in shared feed on article card * Cards fixes and tweaks * Apply Brian's requested changes in his branch/PR and include CV2-4885 scope * testing against api branch of same name * sha512 instead of sha1 in package lock file * Fix workspace ClusterCard rating in Sandbox; Fix ClusterCard unit test; Appease a couple of warnings * improve sandbox cluster card to show zero requests in tipline lists and always show requests in shared feed * include the Articles concept in cluster card sandbox --------- Co-authored-by: Alexandre Amorim <[email protected]> * Ticket CV2-4629: Managing articles from item page (#1999) Managing articles from item page. --------- Co-authored-by: Alexandre Amorim <[email protected]> Co-authored-by: Brian Fleming <[email protected]> Co-authored-by: Caio <[email protected]> * Fixing things I found on the way * Fragments and fixes * More fixes * Show error on blur for article forms * Create fact-check from item page and attach to item in one go * Create explainer from item page and add it to the item * Replace fact-check for item * Articles review cleanup (#2001) * share some more styles for cards with new media article card * tweak slideout width * update copy on articles list alert * Add word article to slideout titles * better cursor behavior for cards * Fix * Reverting package-lock.json * Language should be required for both Explainer and Fact-check * Feature/cv2 4630 remove article from item (#2000) * created remove article button * button with confirmation modal * Added Remove Article button, currently just for Explainer, will add Fact Check next * added remove fact check by updating claim description project media to null * fixed styling issues after fixing merge conflict * wrong type for id * Test/CV2-4883: integration tests for articles (#2003) Delete/Update the current failing tests for claim and fact-check Create fact-check from the list view and make sure it appears in the list after reload Create explainer from the list view and make sure it appears in the list after reload Add fact-check to an item on the item page and go to report page Add explainer to an item on the item page Add new classes/id's for the tests Reference:CV2-4883 * [full ci] * Moar articles review cleanup (#2005) * Use consistent empty state for tags in setttings area and make table look like the rules table more * Clean up privacy requests while reviewing Articles [CV2-4874] * tweak empty annotations message * update scrolling on articles list * add tooltip for remove article button * Change bulk actions language and limit count to the button so the user isn't confused with the pager * add cancel button to existing article slideout * add scroll indicator to articles list on media cluster page * no need to close drawer on media item pages now that we have 2 columns * All to all media in nav and header * minor updates to new fact-check slideout to better match designs * Refactoring/cv2 4882 use articlecard (#1998) * Use ArticleCard in article lists powered by Search component * Play it again, Travis * WIP * Imported and Published articles lists * small copy consistency updates for claim & fact-check wording --------- Co-authored-by: Brian Fleming <[email protected]> * Articles: Auto-refresh (#2004) Implement auto-refresh for all article-related operations. Namely: - [x] Auto-refresh fact-checks list when a new standalone fact-check is created (if it matches current filters/sort) - [x] Auto-refresh explainers list when a new standalone explainer is created (if it matches current filters/sort) - [x] Auto-refresh “articles” tab counter, on the item page, when an article is added/created/removed to/from an item - [x] Fact-check added - [x] Fact-check removed - [x] Fact-check created - [x] Explainer added - [x] Explainer removed - [x] Explainer created - [x] Auto-refresh “articles” tab contents, on the item page, when an article is added/created/removed to/from an item - [x] First fact-check added (no articles) - [x] Fact-check added when item already has explainers - [x] Fact-check removed - [x] Fact-check created - [x] Explainer added (first) - [x] Explainer added (not first) - [x] Explainer removed - [x] Explainer created - [x] Auto-refresh the fact-check card on the item page when fact-check is edited - [x] Auto-refresh the fact-check card on the list view when fact-check is edited - [x] Auto-refresh the explainer card on the item page when explainer is edited - [x] Auto-refresh the explainer card on the list view when explainer is edited - [x] Auto-disable “create fact-check” button on item page after fact-check is added - [x] Auto-disable “create fact-check” button on item page after fact-check is created - [x] Auto-enable “create fact-check” button on item page after fact-check is removed - [x] Auto-refresh article card and form when report is published Because of [this bug](facebook/relay#2049) in the Relay version we use, not all auto-refresh operations work in `development` mode. So, if you really want to test all operations above, you need to move to `production` mode, here: https://github.com/meedan/check-web/blob/feature/CV2-4915-articles-auto-refresh/gulpfile.js#L118. Even this way, not all Relay update operations are possible using Relay Compat, so for such cases I had to use callbacks. References: CV2-4441 (epic) and CV2-4915 (ticket). * [full ci] * Refactoring/cv2 4944 cleanup code (#2010) * Cleanup WIP * Display status report button disabled in slideout * Show report status button in slideout; Click report status icon in card to edit * Remove unwanted comments --------- Co-authored-by: Daniele Valverde <[email protected]> Co-authored-by: Caio Almeida <[email protected]> Co-authored-by: Brian Fleming <[email protected]> Co-authored-by: Brian Fleming <[email protected]> Co-authored-by: Sarah Kehoe <[email protected]> Co-authored-by: Sawy <[email protected]>
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Description
Adds edit to the create form
Info added to top of form, Saves on blur, added cds component for Rating dropdown to display fact check rating, updated ArticleCard to take in as props additional information that the edit form will need. This might need to be added to as these components are properly hooked up
References: CV2-4498
Type of change
How has this been tested?
Please describe the tests that you ran to verify your changes. Provide instructions so we can verify the changes. Please describe whether or not you implemented automated tests.
Things to pay attention to during code review
Added a rough onClick even to open up the editing form, this should be properly connected as now it also opens the form when the user tries adding a tag.
Checklist
propTypesare declared and they use React Hooks and, if data-fetching is required, they use Relay Modern with fragment containers