PORTAL-2480 - creates migration to add expected_cadence to promoted_templates

[MarchandMD]

Description

Adds the expected_cadence text field to the promoted_templates table

issue 2480 of the notification-portal

How Has This Been Tested?

ran flask db upgrade and then flask db downgrade locally

deploy to dev - api action run successful - verifies migration run successfully
DB downgrade action run successful - verifies migration rollback successful

Checklist

• I have assigned myself to this PR
• PR has an appropriate title: #9999 - What the thing does
• PR has a detailed description, including links to specific documentation
• I have added the appropriate labels 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 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
• [na] I have added tests that prove my fix is effective or that my feature works. Testing guidelines
• I have ensured the latest main 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
• [na] The ticket was moved into the DEV test column when I began testing this change

[ianperera]

Looks good so far,

1. What do you think about adding an Index for it?
2. Making it enum type instead of text?

[MarchandMD]

1. I don't believe the promoted_templates table will be queried frequently enough to necessitate this index, though I'd listen to arguments in favor of one
2. Considering the values will be similar to: daily, weekly, monthly, etc, I feel like integers wouldn't be appropriate.

@ianperera

[ianperera]

1. I don't believe the promoted_templates table will be queried frequently enough to necessitate this index, though I'd listen to arguments in favor of one
2. Considering the values will be similar to: daily, weekly, monthly, etc, I feel like integers wouldn't be appropriate.

@ianperera

typo, I meant enum.

[MarchandMD]

Enum would be an unnecessary obfuscation, IMO. The idea of an "expected cadence" is already somewhat opaque, and considering all the potential values (daily, weekly, monthly, quarterly, one-time, adhoc, etc) the most informative and simplest way to store this information for each record is it's actual text value, as opposed to a representation of its value. The reason being, if an expected cadence for a single record is needed, we as engineers would be able to observe a single record and determine its expected cadence without needing to reference the base model to decipher its value from an enum key. IMO.

[cris-oddball]

@MarchandMD before you ask for more approvals, please update the title to follow this convention (necessary for the release notes): PORTAL-2480 - blah blah

Also, this needs to be deployed to Dev to be able to merge (please use the branch name for both the 'workflow' and the 'branch name' in the action, so that the deploy properly links back to this PR) and then run the DB downgrade action, and both of those runs linked in your testing notes. Happy to help if that's unclear. Thank you!

[ianperera]

Enum would be an unnecessary obfuscation, IMO. The idea of an "expected cadence" is already somewhat opaque, and considering all the potential values (daily, weekly, monthly, quarterly, one-time, adhoc, etc) the most informative and simplest way to store this information for each record is it's actual text value, as opposed to a representation of its value. The reason being, if an expected cadence for a single record is needed, we as engineers would be able to observe a single record and determine its expected cadence without needing to reference the base model to decipher its value from an enum key. IMO.

as engineers, if we document the enum values clearly, it would be easy to understand.

[MarchandMD]

@cris-oddball With the help of you and Evan I was able to successfully perform both actions.

The PR description has been updated with links to both of those runs.

Please LMK if there's anything else I can do to expedite this PR! Thank you!

[cris-oddball]

@MarchandMD thank you! If you want the llamas to review, you'll need to ask for that review. I think both Ian and Nathan are in our engineering channel and they can ask for you - see what they say?

[kalbfled]

There should be corresponding changes in app/models.py::PromotedTemplate.

[ianperera]

LGTM