action link on newline working with tests

department-of-veterans-affairs · Aug 16, 2024 · 1c48e95 · 1c48e95
1 parent c624c4a
commit 1c48e95
Show file tree

Hide file tree

Showing 5 changed files with 155 additions and 55 deletions.
diff --git a/.github/pull_request_template.md b/.github/pull_request_template.md
@@ -0,0 +1,49 @@
+<!--
+Before you open this PR, make sure that you review and are taking steps to follow [the Definition of Mergeable in our team agreement](https://docs.google.com/document/d/1nwZIF_lydPWfvixxZlQLNt4nqy3Qp13pHQnMcYJjTqE/edit#heading=h.6mnhaqm79e12). You may need to scroll down to that subheader.
+-->
+
+# Description
+<!--
+Please include a summary of the change and which issue is fixed. Please also include relevant motivation and context. List any dependencies that are required for this change.
+-->
+
+issue #(issue)
+
+## Type of change
+
+Please check the relevant option(s).
+
+- [ ] Bug fix (non-breaking change which fixes an issue)
+- [ ] Hotfix (quick fix for an urgent bug or issue)
+- [ ] New feature (non-breaking change which adds functionality)
+- [ ] Breaking change (fix or feature that would cause existing functionality to not work as expected)
+- [ ] This change requires a documentation update
+- [ ] Documentation changes only
+
+## How Has This Been Tested?
+<!--
+Please describe the tests that you ran to verify your changes. Provide instructions so we can reproduce. Please also list any relevant details for your test configuration
+How has this been tested? (e.g. Unit tests, Tested locally, Tested as a GitHub action, Depoyed to dev, etc)  
+Please provide relevant information and / or links to demonstrate the functionality or fix. (e.g. screenshots, link to deployment, regression test results, etc)
+-->
+
+
+
+## Checklist
+
+- [ ] I have assigned myself to this PR
+- [ ] PR has an [appropriate title](https://github.com/department-of-veterans-affairs/vanotify-team/blob/master/Engineering/team_agreement.md#naming-prs): #9999 - What the thing does
+- [ ] PR has a detailed description, including links to specific documentation
+- [ ] I have added the [appropriate labels](https://github.com/department-of-veterans-affairs/notification-api/blob/master/.github/release.yaml) to the PR.
+- [ ] I did not remove any parts of the template, such as checkboxes even if they are not used
+- [ ] My code follows the [style guidelines](https://github.com/department-of-veterans-affairs/vanotify-team/blob/master/Process/style_guide.md) of this project
+- [ ] I have performed a self-review of my own code
+- [ ] I have commented my code, particularly in hard-to-understand areas
+- [ ] I have made corresponding changes to any documentation
+- [ ] My changes generate no new warnings
+- [ ] I have added tests that prove my fix is effective or that my feature works. [Testing guidelines](https://github.com/department-of-veterans-affairs/vanotify-team/blob/master/Process/testing_guide.md)
+- [ ] I have ensured the latest master is merged into my branch and all checks are green prior to review
+- [ ] New and existing unit tests pass locally with my changes
+- [ ] Any dependent changes have been merged and published in downstream modules
+- [ ] The ticket is now moved into the DEV test column
+- [ ] I have added a bullet for this work to the Engineering Key Wins slide for review
diff --git a/.gitignore b/.gitignore
@@ -65,3 +65,6 @@ target/
 
 # Linux and Mac Dev Environment
 .DS_Store
+
+# pyenv
+.python-version
diff --git a/.talismanrc b/.talismanrc
@@ -1,3 +1,6 @@
 fileignoreconfig:
+- filename: .github/pull_request_template.md
+  checksum: c20998c896b9d321343bc4f45b5b9f04ec37ddbee7d823101e052972a5b6be00
 - filename: notifications_utils/sanitise_text.py
   checksum: 96cf794e49031d454616ae36522a62082e68e3019ba4533084cebf6a6ce0ec0d
+version: "1.0"
diff --git a/notifications_utils/formatters.py b/notifications_utils/formatters.py
@@ -11,7 +11,7 @@
 from notifications_utils.sanitise_text import SanitiseSMS
 import smartypants
 
-
+PARAGRAPH_STYLE = 'Margin: 0 0 20px 0; font-size: 16px; line-height: 25px; color: #323A45;'
 LINK_STYLE = 'word-wrap: break-word; color: #004795;'
 
 OBSCURE_WHITESPACE = (
@@ -288,29 +288,78 @@ def normalise_whitespace(value):
     return ' '.join(strip_and_remove_obscure_whitespace(value).split())
 
 
-def insert_action_link(value: str) -> str:
-    # print('\nhere - insert_action_link')
+def get_action_links(html: str) -> list[str]:
+    """Get the action links from the html email body and return them as a list. (insert_action_link helper)"""
+    # set regex to find action link in html, should look like this:
+    # &gt;&gt;<a ...>link_text</a>
     action_link_regex = re.compile(
-        r'(>|(&gt;)){2}(<a style=".+?" href=".+?"( title=".+?")? target="_blank">)'
+        r'(>|(&gt;)){2}(<a style=".+?" href=".+?"( title=".+?")? target="_blank">)(.*?</a>)'
     )
 
-    # set image link to proper environment
-    link_env = 'dev'
+    return re.findall(action_link_regex, html)
+
+
+def get_img_link() -> str:
+    """Get action link image url for the current environment. (insert_action_link helper)"""
+    img_link = 'https://dev-va-gov-assets.s3-us-gov-west-1.amazonaws.com/img/vanotify-action-link.png'
+
     if os.environ.get('NOTIFY_ENVIRONMENT') == 'production':
-        link_env = 'prod'
+        img_link = 'https://prod-va-gov-assets.s3-us-gov-west-1.amazonaws.com/img/vanotify-action-link.png'
     elif os.environ.get('NOTIFY_ENVIRONMENT') in ['staging', 'performance']:
-        link_env = 'staging'
+        img_link = 'https://staging-va-gov-assets.s3-us-gov-west-1.amazonaws.com/img/vanotify-action-link.png'
 
-    img_link = f'https://{link_env}-va-gov-assets.s3-us-gov-west-1.amazonaws.com/img/vanotify-action-link.png'
+    return img_link
 
-    action_link_list = re.findall(action_link_regex, value)
 
-    if action_link_list:
-        for item in action_link_list:
-            # item values 0 and 1 will be '&gt;'
-            # item 2 is the <a ...> tag info
-            action_link = f'{item[2]}<img src="{img_link}" alt="action img"> '
-            value = value.replace(''.join(item), action_link)
+def insert_action_link(value: str) -> str:
+    """
+    Finds an action link and replaces it with the desired format. The action link is placed on it's own line, the link
+    image is inserted into the link, and the styling is updated appropriately.
+    """
+    # common html used
+    p_start = f'<p style="{PARAGRAPH_STYLE}">'
+    p_end = '</p>'
+
+    action_link_list = get_action_links(value)
+
+    img_link = get_img_link()
+
+    for item in action_link_list:
+        # Puts the action link in a new <p> tag with appropriate styling.
+        # item[0] and item[1] values will be '&gt;' symbols
+        # item[2] is the html link <a ...> tag info
+        # item[-1] is the link text and end of the link tag </a>
+        action_link = (
+            f'{item[2]}<img src="{img_link}" alt="call to action img" '
+            f'style="vertical-align: middle;"> <b>{item[-1][:-4]}</b></a>'
+        )
+
+        action_link_p_tags = f'{p_start}{action_link}{p_end}'
+
+        # get the text around the action link if there is any
+        before_link, after_link = value.split("".join(item))
+
+        # value is the converted action link if there's nothing around it, otherwise <p> tags will need to be
+        # closed / open around the action link
+        if before_link == p_start and after_link == p_end:
+            # action link exists on its own, unlikely to happen
+            value = action_link_p_tags
+        elif before_link.endswith(p_start) and after_link.startswith(p_end):
+            # an action link on it's own line, as it should be
+            value = f'{before_link}{action_link}{after_link}'
+        elif before_link.endswith(p_start):
+            # action link is on a newline, but has something after it on that line
+            value = f'{before_link}{action_link}{p_end}{p_start}{after_link}'
+        elif after_link == p_end:
+            # paragraph ends with action link
+            value = f'{before_link}{"</p>" if "<p" in before_link else ""}{action_link_p_tags}'
+        else:
+            # there's text before and after the action link within the paragraph
+            value = (
+                f'{before_link}{"</p>" if "<p" in before_link else ""}'
+                f'{action_link_p_tags}'
+                f'{p_start}{after_link}'
+            )
 
     return value
 

diff --git a/tests/test_template_types.py b/tests/test_template_types.py
@@ -7,7 +7,7 @@
 from markupsafe import Markup
 from freezegun import freeze_time
 
-from notifications_utils.formatters import LINK_STYLE, unlink_govuk_escaped
+from notifications_utils.formatters import LINK_STYLE, PARAGRAPH_STYLE, unlink_govuk_escaped
 from notifications_utils.template import (
     Template,
     HTMLEmailTemplate,
@@ -39,19 +39,18 @@ def test_pass_through_renderer():
             'line one\nline two with ((name))\n\nnew paragraph',
             {'name': 'bob'},
             (
-                '<p style="Margin: 0 0 20px 0; font-size: 16px; line-height: 25px; color: #323A45;">line one<br />'
-                'line two with bob</p>'
-                '<p style="Margin: 0 0 20px 0; font-size: 16px; line-height: 25px; color: #323A45;">new paragraph</p>'
+                f'<p style="{PARAGRAPH_STYLE}">line one<br />line two with bob</p>'
+                f'<p style="{PARAGRAPH_STYLE}">new paragraph</p>'
             ),
         ),
         (
             '>>[action](https://example.com/foo?a=b)',
             {},
             (
-                '<p style="Margin: 0 0 20px 0; font-size: 16px; line-height: 25px; color: #323A45;">'
-                f'<a style="{LINK_STYLE}" href="https://example.com/foo?a=b" target="_blank">'
+                f'<p style="{PARAGRAPH_STYLE}"><a style="{LINK_STYLE}" href="https://example.com/foo?a=b" '
+                'target="_blank">'
                 '<img src="https://dev-va-gov-assets.s3-us-gov-west-1.amazonaws.com/img/vanotify-action-link.png" '
-                'alt="action img"> action</a></p>'
+                'alt="call to action img" style="vertical-align: middle;"> <b>action</b></a></p>'
             )
         ),
         (
@@ -66,82 +65,79 @@ def test_pass_through_renderer():
                 '<h1 style="Margin: 0 0 20px 0; padding: 0; font-size: 32px; line-height: 35px; font-weight: bold; '
                 'color: #323A45;">foo</h1><h2 style="Margin: 0 0 15px 0; padding: 0; line-height: 26px; color: #323A45;'
                 'font-size: 24px; font-weight: bold; font-family: Helvetica, Arial, sans-serif;">Bar</h2>'
-                '<p style="Margin: 0 0 20px 0; font-size: 16px; line-height: 25px; color: #323A45;">The quick brown fox'
-                f'<br /><a style="{LINK_STYLE}" href="https://example.com" target="_blank">'
+                f'<p style="{PARAGRAPH_STYLE}">The quick brown fox<br /></p>'
+                f'<p style="{PARAGRAPH_STYLE}"><a style="{LINK_STYLE}" href="https://example.com" target="_blank">'
                 '<img src="https://dev-va-gov-assets.s3-us-gov-west-1.amazonaws.com/img/vanotify-action-link.png" '
-                'alt="action img"> the action_link-of doom</a></p>'
+                'alt="call to action img" style="vertical-align: middle;"> <b>the action_link-of doom</b></a></p>'
             ),
         ),
         (
             'text before link\n\n>>[great link](http://example.com)\n\ntext after link',
             {},
             (
-                '<p style="Margin: 0 0 20px 0; font-size: 16px; line-height: 25px; color: #323A45;">text before link'
-                '</p>'
-                '<p style="Margin: 0 0 20px 0; font-size: 16px; line-height: 25px; color: #323A45;">'
-                f'<a style="{LINK_STYLE}" href="http://example.com" target="_blank">'
+                f'<p style="{PARAGRAPH_STYLE}">text before link</p>'
+                f'<p style="{PARAGRAPH_STYLE}"><a style="{LINK_STYLE}" href="http://example.com" target="_blank">'
                 '<img src="https://dev-va-gov-assets.s3-us-gov-west-1.amazonaws.com/img/vanotify-action-link.png" '
-                'alt="action img"> great link</a></p>'
-                '<p style="Margin: 0 0 20px 0; font-size: 16px; line-height: 25px; color: #323A45;">text after link'
-                '</p>'
+                'alt="call to action img" style="vertical-align: middle;"> <b>great link</b></a></p>'
+                f'<p style="{PARAGRAPH_STYLE}">text after link</p>'
             )
         ),
         (
             'action link: &gt;&gt;[Example](http://example.com)\nanother link: [test](https://example2.com)',
             {},
             (
-                '<p style="Margin: 0 0 20px 0; font-size: 16px; line-height: 25px; color: #323A45;">action link: '
-                f'<a style="{LINK_STYLE}" href="http://example.com" target="_blank">'
+                f'<p style="{PARAGRAPH_STYLE}">action link: </p>'
+                f'<p style="{PARAGRAPH_STYLE}"><a style="{LINK_STYLE}" href="http://example.com" target="_blank">'
                 '<img src="https://dev-va-gov-assets.s3-us-gov-west-1.amazonaws.com/img/vanotify-action-link.png" '
-                'alt="action img"> Example</a><br />'
+                'alt="call to action img" style="vertical-align: middle;"> <b>Example</b></a></p>'
+                f'<p style="{PARAGRAPH_STYLE}"><br />'
                 f'another link: <a style="{LINK_STYLE}" href="https://example2.com" target="_blank">test</a></p>'
             )
         ),
         (
             'action link: &gt;&gt;[grin](http://example.com) another action link: >>[test](https://example2.com)',
             {},
             (
-                '<p style="Margin: 0 0 20px 0; font-size: 16px; line-height: 25px; color: #323A45;">action link: '
-                f'<a style="{LINK_STYLE}" href="http://example.com" target="_blank">'
+                f'<p style="{PARAGRAPH_STYLE}">action link: </p>'
+                f'<p style="{PARAGRAPH_STYLE}"><a style="{LINK_STYLE}" href="http://example.com" target="_blank">'
                 '<img src="https://dev-va-gov-assets.s3-us-gov-west-1.amazonaws.com/img/vanotify-action-link.png" '
-                'alt="action img"> grin</a>'
-                f' another action link: <a style="{LINK_STYLE}" href="https://example2.com" target="_blank">'
+                'alt="call to action img" style="vertical-align: middle;"> <b>grin</b></a></p>'
+                f'<p style="{PARAGRAPH_STYLE}"> another action link: </p>'
+                f'<p style="{PARAGRAPH_STYLE}"><a style="{LINK_STYLE}" href="https://example2.com" target="_blank">'
                 '<img src="https://dev-va-gov-assets.s3-us-gov-west-1.amazonaws.com/img/vanotify-action-link.png" '
-                'alt="action img"> test</a>'
-                '</p>'
+                'alt="call to action img" style="vertical-align: middle;"> <b>test</b></a></p>'
             )
         ),
         (
             'text before && link &gt;&gt;[Example](http://example.com) text after & link',
             {},
             (
-                '<p style="Margin: 0 0 20px 0; font-size: 16px; line-height: 25px; color: #323A45;">'
-                'text before &amp;&amp; link '
-                f'<a style="{LINK_STYLE}" href="http://example.com" target="_blank">'
+                f'<p style="{PARAGRAPH_STYLE}">text before &amp;&amp; link </p>'
+                f'<p style="{PARAGRAPH_STYLE}"><a style="{LINK_STYLE}" href="http://example.com" target="_blank">'
                 '<img src="https://dev-va-gov-assets.s3-us-gov-west-1.amazonaws.com/img/vanotify-action-link.png" '
-                'alt="action img"> Example</a> text after &amp; link</p>'
+                'alt="call to action img" style="vertical-align: middle;"> <b>Example</b></a></p>'
+                f'<p style="{PARAGRAPH_STYLE}"> text after &amp; link</p>'
             )
         ),
         (
-            'text before >> link &gt;&gt;[great Example](http://example.com) text after >>link',
+            'text before >> link &gt;&gt;[great action](http://example.com) text after >>link',
             {},
             (
-                '<p style="Margin: 0 0 20px 0; font-size: 16px; line-height: 25px; color: #323A45;">'
-                'text before &gt;&gt; link '
-                f'<a style="{LINK_STYLE}" href="http://example.com" target="_blank">'
+                f'<p style="{PARAGRAPH_STYLE}">text before &gt;&gt; link </p>'
+                f'<p style="{PARAGRAPH_STYLE}"><a style="{LINK_STYLE}" href="http://example.com" target="_blank">'
                 '<img src="https://dev-va-gov-assets.s3-us-gov-west-1.amazonaws.com/img/vanotify-action-link.png" '
-                'alt="action img"> great Example</a> text after &gt;&gt;link</p>'
+                'alt="call to action img" style="vertical-align: middle;"> <b>great action</b></a></p>'
+                f'<p style="{PARAGRAPH_STYLE}"> text after &gt;&gt;link</p>'
             )
         ),
         (
             'text >> then [item] and (things) then >>[action](link)',
             {},
             (
-                '<p style="Margin: 0 0 20px 0; font-size: 16px; line-height: 25px; color: #323A45;">text &gt;&gt; then'
-                ' [item] and (things) then '
-                f'<a style="{LINK_STYLE}" href="link" target="_blank">'
+                f'<p style="{PARAGRAPH_STYLE}">text &gt;&gt; then [item] and (things) then </p>'
+                f'<p style="{PARAGRAPH_STYLE}"><a style="{LINK_STYLE}" href="link" target="_blank">'
                 '<img src="https://dev-va-gov-assets.s3-us-gov-west-1.amazonaws.com/img/vanotify-action-link.png" '
-                'alt="action img"> action</a></p>'
+                'alt="call to action img" style="vertical-align: middle;"> <b>action</b></a></p>'
             )
         ),
     ],