nasa / mmt

NASA's Metadata Management Tool.
Apache License 2.0
87 stars 36 forks source link

Mmt 3877b: Error When Cloning and Editing a Draft Collection #1297

Closed mandyparson closed 1 month ago

mandyparson commented 2 months ago

Overview

What is the bug?

Errors occur during two particular use cases:

Collections >> Collection >> Clone >> 'Cannot destructure property \'nativeId\' of \'Tr\' as it is null.' (this error comes from DraftPage.jsx line 90 when MMT tries to ask for the draft but the draft doesn't exist in CMR yet)

Collections >> Collection >> Clone >> Edit >> Save & Publish >> 'Cannot destructure property \'granules\' of \'Cr\' as it is null.' (this error comes from PublishPreview.jsx line 101 when MMT tries to ask for the published record but it doesn't exist in CMR yet)

Please summarize the feature or fix.

Initially, the thought was that we should use useHistory for this bug. If a user is coming from publishpreview or is coming from draftpage, then render a refresh banner. But there are two issues with this: 1) useHistory is not supported for react-router v6 or above. https://stackoverflow.com/questions/63471931/using-history-with-react-router-dom-v6 it is recommended that you use useNavigation. If you looke up history npm, the use cases are only for actions and blocking certain transitions. There does not appear to be an array of pash urls saved for reference. 2) Even if I could access history, I don't believe it is wise to use a url as a metric for what error to throw. What if a user does another type of action on PublishPreview or DraftPage that causes an error? It would throw a refresh banner no matter what.

Rather than use useHistory, I have opted to render a refresh banner based on those repeatable errors I listed above.

What is the Solution?

Adding knownCMRLagErrors to ErrorBanner

What areas of the application does this impact?

ErrorBanner.jsx

Testing

Reproduction steps

  1. From a collection, click clone until you receive the refresh page error message. Let me know if you get 'record does not exist'
  2. From a clone, click edit, add in a short name and entry title, then click Save & Publish until you receive the refresh page error message. Again, let me know if you receive any other error.
  3. Try to trigger error banners in any other areas around the app. I want to be sure this fix doesn't accidentally affect other areas that it shouldn't.

Attachments

Please include relevant screenshots or files that would be helpful in reviewing and verifying this change.

Checklist

codecov-commenter commented 2 months ago

Codecov Report

Attention: Patch coverage is 75.00000% with 3 lines in your changes missing coverage. Please review.

Project coverage is 97.65%. Comparing base (960eb4f) to head (31d009b).

Files with missing lines Patch % Lines
...ic/src/js/components/DraftPreview/DraftPreview.jsx 50.00% 1 Missing :warning:
...rc/js/components/PublishPreview/PublishPreview.jsx 50.00% 1 Missing :warning:
static/src/js/pages/DraftPage/DraftPage.jsx 50.00% 1 Missing :warning:
Additional details and impacted files ```diff @@ Coverage Diff @@ ## main #1297 +/- ## ========================================== - Coverage 97.70% 97.65% -0.06% ========================================== Files 362 362 Lines 5537 5540 +3 Branches 1144 1157 +13 ========================================== Hits 5410 5410 - Misses 126 129 +3 Partials 1 1 ```

:umbrella: View full report in Codecov by Sentry.
:loudspeaker: Have feedback on the report? Share it here.