-
Notifications
You must be signed in to change notification settings - Fork 16
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
Feature/3543 applicants info position and email header change #2260
Feature/3543 applicants info position and email header change #2260
Conversation
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
-
Tests will need to be updated, there's some tests for the email subjects during the application workflow.
-
Should we have a colon in there, e.g. New application assigned to you: 1234-1234, 0000-0000 since ISSN is a required field we shouldn't need to worry about the case where they're missing.
ISSN is required but not necessarily both of them (it's possible to submit an application with PISSN only)? The |
colon : <--- :) |
Move the applicant's email address into the Notes sticky and ensure it follows the editor down the page.
Add the ISSN into the email subject heading for all notifications defined in notifications.yml
Please don't delete any sections when completing this PR template; instead enter N/A for checkboxes or sections which are not applicable, unless otherwise stated below
See 3543
Categorisation
This PR...
Basic PR Checklist
Instructions for developers:
Instructions for reviewers:
Code Style
No deprecated methods are used
No magic strings/numbers - all strings are in
constants
ormessages
filesES queries are wrapped in a Query object rather than inlined in the code
Where possible our common library functions have been used (e.g. dates manipulated via
dates
)Cleaned up commented out code, etc
Testing
Unit tests have been added/modified
Functional tests have been added/modified
Code has been run manually in development, and functional tests followed locally
Documentation
FeatureMap annotations have been added
Documentation updates - if needed - have been identified and prepared for inclusion into main documentation (e.g. added and highlighted/commented as appropriate to this PR)
Core model documentation has been added to if needed: https://docs.google.com/spreadsheets/d/1lun2S9vwGbyfy3WjIjgXBm05D-3wWDZ4bp8xiIYfImM/edit
Events and consumers documentation has been added if needed: https://docs.google.com/spreadsheets/d/1oIeG5vg-blm2MZCE-7YhwulUlSz6TOUeY8jAftdP9JE/edit
The docs for this branch have been generated and pushed to the doc site (see docs/README.md for details)
Release Readiness
If needed, migration has been created and tested locally
Release sheet has been created, and completed as far as is possible https://docs.google.com/spreadsheets/d/1Bqx23J1MwXzjrmAygbqlU3YHxN1Wf7zkkRv14eTVLZQ/edit
There has been a recent merge up from
develop
(or other base branch). List the dates of the merges up from develop below07/07/2023
Testing
List the Functional Tests that must be run to confirm this feature
https://doaj.github.io/doaj-docs/feature/3543_applicants_info_position_and_email_header_change/testbook/index.html#notifications/notifications/short_notification_view