reason to denial reason

[ArushC]

Pivotal Tracker Link

What this PR does:

Replace the "reason" parameter with "denial_reason". Also updates the name of all relevant instance variables and attributes from "reason" to "denial_reason" to avoid confusion in the code, except in the one case where the @ reason instance variable was being used both for the denial reason and the more information request reason (not implemented yet)—here, I added a new instance variable @reason_request for the more information request in the relevant controller to pass along the information.

IMPORTANT: to reflect the changes in the denial email template (changing {reason} to {denial reason}), the EmailTemplate database needs to be either reseeded or edited manually through the web interface by an admin: just change {reason | ...} to {denial_reason | ...} in the email template. Also, the request more info email template should be changed to use {request_reason | ...} instead of {reason | ...}.

This pull request fixes|implements (pick one...) ______.

Include screenshots, videos, etc.

Who authored this PR?

@ArushC

How should this PR be tested?

• Is there a deploy we can view?
• What do the specs/features test?
• Are there edge cases to watch out for?

Are there any complications to deploying this?

Checklist:

• Has this been deployed to a staging environment or reviewed by a customer?
• Tag someone for code review (either a coach / team member)
• I have renamed the branch to match PivotTracker's suggested one (necessary for BlueJay) (e.g. michael/12345-add-new-feature Any branch name will do as long as the story ID is there. You can use git checkout -b [new-branch-name])

[codecov]

Codecov Report

Attention: 2 lines in your changes are missing coverage. Please review.

Comparison is base (d35ca2c) 81.95% compared to head (092cce2) 81.95%.
Report is 1 commits behind head on main.

Files	Patch %	Lines
app/controllers/teachers_controller.rb	50.00%	1 Missing ⚠️
app/mailers/teacher_mailer.rb	75.00%	1 Missing ⚠️

Additional details and impacted files

@@           Coverage Diff           @@
##             main      #29   +/-   ##
=======================================
  Coverage   81.95%   81.95%           
=======================================
  Files          20       20           
  Lines         654      654           
=======================================
  Hits          536      536           
  Misses        118      118           

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

[fp456]

LGTM. Branch name seems correct, as well the overall quality of the PR.

[perryzjc]

Is this {reason} field still supposed to exist here?

I can notice the {reason} at the Allowed Tags is changed to {{denial_reason}}, which is expected:

[perryzjc]

For other renaming issues, I think it's fine for now. We can fix it when implementing request_reason feature.

[ArushC]

@perryzjc the reason field is not supposed to exist where you pointed out. But this can be updated manually through the user interface. It is probably much easier than generating a custom migration to update only that single field. I mentioned in the PR description:

"IMPORTANT: to reflect the changes in the denial email template (changing {reason} to {denial reason}), the EmailTemplate database needs to be either reseeded or edited manually through the web interface by an admin: just change {reason | ...} to {denial_reason | ...} in the email template. Also, the request more info email template should be changed to use {request_reason | ...} instead of {reason | ...}."

[ArushC]

I will merge the changes. We will just have to remember to mention that the reason->denial_reason update will have to be made manually through the UI on the actual application.