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

918 Added tags and notes to docket alerts #2431

Merged
merged 8 commits into from Jan 4, 2023

Conversation

albertisfu
Copy link
Contributor

Added tags and notes to docket alerts as required in #918

  • Added a new method get_docket_notes_and_tags_by_user to get the user notes and tags for a docket and add them to the docket alert email.

  • For the HTML version we show the tags with links, so it was needed to add the username to the email context in order to to generate the tag URL.

  • For the plain text version, we only show the tag name.

  • Added documentation for Favorites on the same page of Tags, now /help/tags-favorites/

  • Docket alert screenshot updated in /help/alerts/ to show the latest additions, Your notes, and Your Tags.

Here are some screenshots:

Docket with Notes and Tags:
Screen Shot 2022-12-29 at 21 27 04

Docket with Tags but no Notes
Screen Shot 2022-12-29 at 21 26 43

Docket with Notes but no Tags
Screen Shot 2022-12-29 at 21 25 33

Docket without Tags and Notes
Screen Shot 2022-12-29 at 21 26 00

The Learn more link goes to /help/tags-favorites/

  • Fixed the subject of webhook_still_disabled_email, I noticed it lacked the word "webhook" before the event type.
  • Fixed the test_send_search_alert_webhooks_rates that checks all the rates for sending search alerts. And monthly alerts cannot be run on the 29th, 30th, or 31st, so it was failing (since today is 29th) added time_machine to mock the date.

Let me know what you think.

- Monthly alerts cannot be run on the 29th, 30th or 31st, so this test was failing (today is 29) added time_machine to mock the date.
Copy link
Member

@mlissner mlissner left a comment

Choose a reason for hiding this comment

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

These emails look great. I think this will finally drive some adoption among our users of these features.

A handful of comments, and then a couple things:

  1. For alert-example.png, you changed the dimensions, to make it wider and taller. The height will need to be adjusted in the HTML where it's viewed, and the width should probably be the same as before (but it's worth checking the max-width of the container where it's in, and setting it to that).

  2. For all your screenshots, can you update the note to say something like this, instead of Lorem Ipsum:

    This the case about black lung personal responsibility among minor miners. The plaintiffs, a small class of coal minors, believe that minors should not be responsible for the damage to their lungs. The defendants claim teenage vaping as a defense and that the parents of all minors signed waivers on their behalf.

Thanks.

cl/alerts/tasks.py Outdated Show resolved Hide resolved
cl/alerts/templates/docket_alert_email.html Outdated Show resolved Hide resolved
cl/alerts/templates/docket_alert_email.txt Outdated Show resolved Hide resolved
cl/simple_pages/templates/help/tags_help.html Show resolved Hide resolved
cl/simple_pages/templates/help/tags_help.html Outdated Show resolved Hide resolved
cl/simple_pages/templates/help/tags_help.html Outdated Show resolved Hide resolved
cl/simple_pages/templates/help/tags_help.html Outdated Show resolved Hide resolved
cl/simple_pages/templates/help/tags_help.html Outdated Show resolved Hide resolved
cl/simple_pages/templates/help/tags_help.html Outdated Show resolved Hide resolved
cl/simple_pages/templates/help/tags_help.html Outdated Show resolved Hide resolved
@albertisfu
Copy link
Contributor Author

Seems there is a mypy, error, checking it...

@albertisfu
Copy link
Contributor Author

These changes are completed. Also updated screenshots were there is a Note, using the following text:

This the case about black lung personal responsibility among minor miners. The plaintiffs, a small class of coal minors, believe that minors should not be responsible for the damage to their lungs. The defendants claim teenage vaping as a defense and that the parents of all minors signed waivers on their behalf.

And fixed screenshots size in the HTML.

Let me know if there is anything else here.

Thank you!

cl/alerts/tasks.py Outdated Show resolved Hide resolved
Copy link
Member

@mlissner mlissner left a comment

Choose a reason for hiding this comment

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

Looks good. One comment for you, below.

I also noticed a couple typos in the screenshots. If it's easy to re-do them (like, less than five minutes), this text should be better (sorry):

This case is about laughing disease liability among minor miners. The plaintiffs, a small class of coal miners, believe that minors should not be responsible for the unstoppable laughing caused by gases in the mines. The defendants claim teenage vaping as a defense and that the parents of all minor miners signed a waiver on their behalf.

@albertisfu
Copy link
Contributor Author

Yeah, I've applied the requested change and replaced the text in the screenshots.
Thanks!

@mlissner mlissner merged commit 2944801 into main Jan 4, 2023
@mlissner mlissner deleted the 918-add-tags-and-notes-to-docket-alerts branch January 4, 2023 19:49
@mlissner
Copy link
Member

mlissner commented Jan 4, 2023

Thanks, this is great!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
Status: Done
Status: Done
Development

Successfully merging this pull request may close these issues.

None yet

2 participants