harvard-lil / perma

Indelible links
423 stars 71 forks source link

Tweak error messages and upload your own behavior. #3625

Closed rebeccacremona closed 1 month ago

rebeccacremona commented 1 month ago

In this PR, I'm doing three uncontroversial things, and one potentially controversial one.

Uncontroversial

1) While doing QA of the new Vue dashboard, I noticed that some of our error messages are unnecessarily terse or are otherwise a little clumsy. I tweaked them to suit my personal sensibilities; please feel free to suggest alternatives!

2) I noticed that the upload-your-own form wasn't handling titles and descriptions correctly: they should always be optional, and you should get the chance to supply them both when uploading fresh, or when replacing a failed capture.

image

3) I realized that I was wrong last week, when I said that the option to upload your own should not appear when you enter an invalid URL. That is in fact not correct: many valid URLs, like https://afdc.energy.gov/stations#/analyze?country=US&region=US-VA&fuel=ELEC, currently fail validation, and our users rely on this workaround. See LIL-836.

Potentially controversial

This PR significantly improved error handling.

But, in the process, it switched to unstyled messages... which I personally am finding difficult to parse, because the messages are long.

Here, I tried reintroducing a little styling.

I can easily revert it, if we prefer the unstyled, one-line version.

Examples:

image image image image

rebeccacremona commented 1 month ago

(I also think it is a goofy we have two versions, a bunch that start with "We're unable to create your Perma Link" and a bunch that start with "Perma can't create this link." I am tempted to fix that up too... but am not sure.)

codecov[bot] commented 1 month ago

Codecov Report

Attention: Patch coverage is 66.66667% with 1 line in your changes missing coverage. Please review.

Project coverage is 69.70%. Comparing base (2e7354b) to head (6ccc5c5). Report is 14 commits behind head on develop.

Files with missing lines Patch % Lines
perma_web/api/serializers.py 66.66% 1 Missing :warning:
Additional details and impacted files ```diff @@ Coverage Diff @@ ## develop #3625 +/- ## ======================================== Coverage 69.70% 69.70% ======================================== Files 54 54 Lines 7317 7317 ======================================== Hits 5100 5100 Misses 2217 2217 ```

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

jcushman commented 1 month ago

Looks great! Let's do it.

rebeccacremona commented 1 month ago

LGTM. I notice that we vary in using and ' for apostrophes, but maybe we can fix that systematically another time.

Yeah I am the worst about that 😅 . I use straight quotes everywhere.