department-of-veterans-affairs / notification-utils

Notification Utils
MIT License
1 stars 0 forks source link

Implement New Action Link Formatting Element for Portal #152

Closed juliexmartin closed 2 months ago

juliexmartin commented 6 months ago

User Story - Business Need

Clients have asked for a way to emphasize call to action links in email content, like a button. VA design standards does not support a button, but UX has found a promising design element called an action link which they'd like to add to the portal template formatting options.

User Story(ies)

As a VA Notify-onboarded business line, I want to call attention to the primary link in the email So that the Veteran is more likely to click on the link.

Additional Info and Resources

Action link is https://<env>-va-gov-assets.s3-us-gov-west-1.amazonaws.com/img/vanotify-action-link.png

Engineering Checklist

However we are using mistune is awful and it was ignored for this ticket after spending way to long trying to get it to work properly.

Acceptance Criteria

  1. When a VA Notify Portal user adds an action link element into an email template, the email should display the action link icon and the hyperlink specified.
    1. This should apply to when the action link is viewed in both a) email preview mode and b) once the email has been sent.
    2. This should apply to both a) when the action link is by itself, and b) when the action link is in card format.
      1. If the action link is in card format, then it should also utilize the card formatting.
  2. The action link icon and hyperlink should be clickable.
    1. If the action link is in card format, then it should also utilize the card formatting.
    2. Hyperlinks can include personalizations per existing formatting.
Completed work meets the requirements below or alternatives have been reviewed and approved by Product and UX: - UX wants a flexible element that can be used independently on a new line or combined with the existing card element. - If combined with the card element, it would still be the only thing on that line, but would use the styling of the card also. - The action link itself is the combination of the icon followed by a hyperlink. - There can only be 1 hyperlink per action link. This element will not support static text alone or in combination with the hyperlink. - UX would like the entire element (image and hyperlink text) to be clickable, but it's not a deal breaker if only the hyperlink is clickable. - There will only be 1 action link per template ## QA Considerations Once the formatting changes have been made and deployed to Perf, you should be able to update your template to include an action link and check template preview (which uses our API to render). You can then send yourself an email to check the content. ## Potential Dependencies _Leave blank if n/a_ - We don't own the public s3 bucket - va.gov does, so we'll need to get them to add it or figure out how to make our own publicly accessible bucket ## Out of Scope There’s an existing bug with template preview for personalizations in URLs. I’m not asking to fix this as part of this ticket unless as you’re in there it’s easier than expected. Reach out if that’s the case to double check. https://app.zenhub.com/workspaces/va-notify-620d21369d810a00146ed9c8/issues/gh/department-of-veterans-affairs/notification-utils/127
EvanParish commented 3 months ago

This is proving to be trickier than I expected especially after reading the spike documentation. I've tried a few different ways of creating the action link, but haven't had any success so far. I will try working with Michael in the morning to see if we can put our heads together on this.

EvanParish commented 3 months ago

I worked with Michael briefly this morning, but Mistune continues to be a nightmare. The custom lexer I added appears to be working when the notify_email_markdown object is called directly, but doesn't work when called from somewhere else, like get_html_email_body. There seems to be something I'm missing here, so I'm still trying to understand what that is.

I added a unit test that passes when calling notify_email_markdown. I thought that would be sufficient, but it seems I'll need to add some tests to ensure everything is working together as expected.

EvanParish commented 3 months ago

Met with Kyle and there weren't any obvious solutions jumping out at us. Kyle is going to take a look on his own sometime today, and I'll continue to work on it as well. Maybe the Mistunes code has some hints?

EvanParish commented 3 months ago

After looking into it a bit more Kyle managed to help me get un-stuck from the problem I was facing. Now I need to do some testing and confirm the expectation with someone from the Strike team.

EvanParish commented 3 months ago

With how we implemented the lexer I'm currently hitting a problem with it not properly recognizing newlines within text. This is causing test_paragraphs to fail when using the notify_email_markdown function, which uses the updated code. There's another way I can have it work, but it's more "hacky" than I'd like.

EvanParish commented 3 months ago

So it looks like ignoring mistune was the way to go. It might need some styling changes, but I was able to add a template with action links! The image showed in the previews of the email template and they in the email when I sent one to myself.

image

EvanParish commented 3 months ago

Coordinated with Ian and Elissa to make sure the action link is looking how it's supposed to. There's a little more work to do based on their feedback, but I'm hoping to have the PR up Friday.

EvanParish commented 3 months ago

Met with Ian this morning to talk about the expectations for the action link design. I'll have to wait for Elisa to get back to ask her what she meant with the AC that talks about the action link being in "card format."

The new changes and thorough unit testing took a little longer than I expected, so the PR is not up yet. Everything seems to be working locally and most scenarios are covered by the unit tests. I'll deploy and test Monday morning. As long as everything looks good during that testing the PR will be open for review.

EvanParish commented 2 months ago

Found a bug when testing this morning, so I had to clear that up, which deplayed opening the PR. This is carrying over to the new sprint to make sure we have time to QA it before closing it out.

The ticket took a long time in general because it had to be reworked a couple of times to find a solution that would work consistently and not break any existing functionality. The older version of Mistune that we use also does not have very robust documentation, so a lot of work went into going through that packages code to figure out how things work.

cris-oddball commented 2 months ago

QA PASSED

PRs (utils and api) approved and merged; deployed to Perf in Pipieline. Promoted the template that Evan created in Dev to Perf.

Template preview route in Perf:

UTILS-152-image001.png

POST 201 response showing the content with the body:

UTILS-152-image002.png

Received email:

Screenshot 2024-08-20 at 1.27.25 PM.jpg

YouTube review indicates that the link works. Sadly, I had to click the link to test it. And now I have been rickrolled.