-
Notifications
You must be signed in to change notification settings - Fork 3
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
Osdev 1376 closure report. updating auto email response to remove the word 'rejected' #460
Conversation
Updated text to remove the word "Rejected", added link to facility claim policy and clearer instruction to send a Reopen report.
Removed "Rejected", added link to policy, added instructions to submit reopening report.
Added closure report automated email update OSDEV-1376.
📝 Walkthrough📝 WalkthroughWalkthroughThe pull request introduces improvements to the Open Supply Hub project, focusing on enhancing user experience and fixing backend issues. The changes include correcting date validation on the Moderation Queue page, fixing country sorting in the API endpoint, and updating email templates for closure reports. The release notes and email templates have been modified to provide clearer communication, remove the term "Rejected", and add links to the Closure Policy. Additionally, release instructions were updated to ensure proper database migration and indexing during deployment. Changes
Possibly Related PRs
Suggested Reviewers
📜 Recent review detailsConfiguration used: CodeRabbit UI 📒 Files selected for processing (1)
🚧 Files skipped from review as they are similar to previous changes (1)
Thank you for using CodeRabbit. We offer it for free to the OSS community and would appreciate your support in helping us grow. If you find it useful, would you consider giving us a shout-out on your favorite social media? 🪧 TipsChatThere are 3 ways to chat with CodeRabbit:
Note: Be mindful of the bot's finite context window. It's strongly recommended to break down tasks such as reading entire modules into smaller chunks. For a focused discussion, use review comments to chat about specific files and their changes, instead of using the PR comments. CodeRabbit Commands (Invoked using PR comments)
Other keywords and placeholders
CodeRabbit Configuration File (
|
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.
Actionable comments posted: 0
🧹 Nitpick comments (4)
src/django/api/templates/mail/report_result_body.txt (2)
6-6
: Consider adding commas in compound sentencesThe message content is clear but could benefit from better punctuation in compound sentences.
Apply these changes for better readability:
-{% if is_closure and is_rejected %}We did not approve this report and the facility's profile in OS Hub has not been changed. For more details, view our Facility Closure Policy: https://info.opensupplyhub.org/governance-policies.{% endif %}{% if is_closure and is_confirmed %}The facility profile on OS Hub has now been updated and the facility is marked as closed.{% endif %}{% if is_reopening and is_rejected %}We did not approve this report and the facility's profile on OS Hub has not been changed.{% endif %}{% if is_reopening and is_confirmed %}The facility profile on OS Hub has now been updated and the facility is no longer marked as closed.{% endif %} +{% if is_closure and is_rejected %}We did not approve this report, and the facility's profile in OS Hub has not been changed. For more details, view our Facility Closure Policy: https://info.opensupplyhub.org/governance-policies.{% endif %}{% if is_closure and is_confirmed %}The facility profile on OS Hub has now been updated, and the facility is marked as closed.{% endif %}{% if is_reopening and is_rejected %}We did not approve this report, and the facility's profile on OS Hub has not been changed.{% endif %}{% if is_reopening and is_confirmed %}The facility profile on OS Hub has now been updated, and the facility is no longer marked as closed.{% endif %}🧰 Tools
🪛 LanguageTool
[uncategorized] ~6-~6: Use a comma before “and” if it connects two independent clauses (unless they are closely connected and short).
Context: ...y profile on OS Hub has now been updated and the facility is marked as closed.{% end...(COMMA_COMPOUND_SENTENCE_2)
[uncategorized] ~6-~6: Use a comma before “and” if it connects two independent clauses (unless they are closely connected and short).
Context: ...y profile on OS Hub has now been updated and the facility is no longer marked as clo...(COMMA_COMPOUND_SENTENCE_2)
10-10
: Fix grammar and formatting issuesThere are several grammar and formatting issues in the instructions section.
Apply these changes:
-{% if is_rejected %}If you have additional evidence that can be used to verify that the facility is {{ closure_state|lower }}, email the relevant documents to: [email protected]{% endif %}{% if is_closure and is_confirmed %}Should you receive reports that the facility is re-opened in the future, please send us an report clicking the "Report" button (flag icon) on the facility profile and choosing "Report as Reopened".{% endif %} +{% if is_rejected %}If you have additional evidence that can be used to verify that the facility is {{ closure_state|lower }}, email the relevant documents to [email protected]{% endif %}{% if is_closure and is_confirmed %}Should you receive reports that the facility is re-opened in the future, please send us a report by clicking the "Report" button (flag icon) on the facility profile and choosing "Report as Reopened".{% endif %}Changes:
- Removed unnecessary colon before email address
- Fixed article usage: "an report" → "a report"
- Added "by" before "clicking" for better flow
🧰 Tools
🪛 LanguageTool
[typographical] ~10-~10: Do not use a colon (:) before a series that is introduced by a preposition (‘to’). Remove the colon or add a noun or a noun phrase after the preposition.
Context: ...|lower }}, email the relevant documents to: [email protected]{% endif %}{% if ...(RP_COLON)
[misspelling] ~10-~10: Use “a” instead of ‘an’ if the following word doesn’t start with a vowel sound, e.g. ‘a sentence’, ‘a university’.
Context: ...re-opened in the future, please send us an report clicking the “Report” button (fl...(EN_A_VS_AN)
src/django/api/templates/mail/report_result_body.html (2)
13-17
: Ensure consistent spacing in HTML templateThe HTML template has inconsistent spacing between paragraphs and conditions.
Add consistent line breaks between paragraphs and conditions for better maintainability:
Thank you for reporting the facility {{ facility_name }} as {{ closure_state|lower }}. - {% if is_closure and is_rejected %}We did not approve this report and the facility's profile on OS Hub has not been changed. For more details, view our <a href="https://info.opensupplyhub.org/governance-policies">Facility Closure Policy</a>.{% endif %} - {% if is_closure and is_confirmed %}The facility profile on OS Hub has now been updated and the facility is marked as closed.{% endif %} - {% if is_reopening and is_rejected %}We did not approve this report and the facility's profile on OS Hub has not been changed.{% endif %} - {% if is_reopening and is_confirmed %}The facility profile on OS Hub has now been updated and the facility is no longer marked as closed.{% endif %} + + {% if is_closure and is_rejected %} + We did not approve this report, and the facility's profile on OS Hub has not been changed. For more details, view our <a href="https://info.opensupplyhub.org/governance-policies">Facility Closure Policy</a>. + {% endif %} + + {% if is_closure and is_confirmed %} + The facility profile on OS Hub has now been updated, and the facility is marked as closed. + {% endif %} + + {% if is_reopening and is_rejected %} + We did not approve this report, and the facility's profile on OS Hub has not been changed. + {% endif %} + + {% if is_reopening and is_confirmed %} + The facility profile on OS Hub has now been updated, and the facility is no longer marked as closed. + {% endif %}
26-26
: Fix grammar issues in instruction paragraphsThe instruction paragraphs have the same grammar issues as the text template.
Apply these changes:
- If you have additional evidence that can be used to verify that the facility is {{ closure_state|lower }}, email the relevant documents to: [email protected] + If you have additional evidence that can be used to verify that the facility is {{ closure_state|lower }}, email the relevant documents to [email protected] - Should you receive reports that the facility is re-opened in the future, please send us an report clicking the "Report" button (flag icon) on the facility profile and choosing "Report as Reopened" + Should you receive reports that the facility is re-opened in the future, please send us a report by clicking the "Report" button (flag icon) on the facility profile and choosing "Report as Reopened"Also applies to: 31-31
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (3)
doc/release/RELEASE-NOTES.md
(1 hunks)src/django/api/templates/mail/report_result_body.html
(2 hunks)src/django/api/templates/mail/report_result_body.txt
(1 hunks)
🧰 Additional context used
🪛 LanguageTool
src/django/api/templates/mail/report_result_body.txt
[uncategorized] ~6-~6: Use a comma before “and” if it connects two independent clauses (unless they are closely connected and short).
Context: ...y profile on OS Hub has now been updated and the facility is marked as closed.{% end...
(COMMA_COMPOUND_SENTENCE_2)
[uncategorized] ~6-~6: Use a comma before “and” if it connects two independent clauses (unless they are closely connected and short).
Context: ...y profile on OS Hub has now been updated and the facility is no longer marked as clo...
(COMMA_COMPOUND_SENTENCE_2)
[typographical] ~10-~10: Do not use a colon (:) before a series that is introduced by a preposition (‘to’). Remove the colon or add a noun or a noun phrase after the preposition.
Context: ...|lower }}, email the relevant documents to: [email protected]{% endif %}{% if ...
(RP_COLON)
[misspelling] ~10-~10: Use “a” instead of ‘an’ if the following word doesn’t start with a vowel sound, e.g. ‘a sentence’, ‘a university’.
Context: ...re-opened in the future, please send us an report clicking the “Report” button (fl...
(EN_A_VS_AN)
🔇 Additional comments (3)
src/django/api/templates/mail/report_result_body.txt (2)
2-4
: LGTM: Professional greeting and acknowledgment
The updated greeting "Hello" and acknowledgment message are more professional and clearly state the context of the report.
12-12
: LGTM: Professional closing
The closing "Best Regards" is professional and appropriate.
doc/release/RELEASE-NOTES.md (1)
26-26
: LGTM: Clear and comprehensive release note
The release note clearly documents the changes made to the automated emails, including the removal of "Rejected" term and addition of policy links and reopening instructions.
React App | Jest test suite - Code coverage reportTotal: 27.88%Your code coverage diff: 0.00% ▴ ✅ All code changes are covered |
Dedupe Hub App | Unittest test suite - Code coverage reportTotal: 56.14%Your code coverage diff: 0.00% ▴ ✅ All code changes are covered |
Countries App | Unittest test suite - Code coverage reportTotal: 100%Your code coverage diff: 0.00% ▴ ✅ All code changes are covered |
Contricleaner App | Unittest test suite - Code coverage reportTotal: 98.91%Your code coverage diff: 0.00% ▴ ✅ All code changes are covered |
Django App | Unittest test suite - Code coverage reportTotal: 79.96%Your code coverage diff: 0.00% ▴ ✅ All code changes are covered |
…il-response-to-remove-the-word-'rejected'
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.
LGTM
remove spacing
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.
LGTM
Quality Gate passedIssues Measures |
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.
LGTM
Updated the automated email for closure reports (report_result( to remove the term "Rejected" for an improved user experience. Added link to Closure Report policy and instructions to submit a reopening report to provide users with more information on the process.