Skip to content
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

Reason to denial reason #28

Closed
wants to merge 3 commits into from
Closed

Reason to denial reason #28

wants to merge 3 commits into from

Conversation

ArushC
Copy link

@ArushC ArushC commented Feb 16, 2024

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}), either the email template database should be reseeded or the template should be edited manually through the web interface. Simply change reason->denial_reason.

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

Include screenshots, videos, etc.

Who authored this PR?

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])

Copy link

codecov bot commented Feb 16, 2024

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%.

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      #28   +/-   ##
=======================================
  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.

@perryzjc perryzjc temporarily deployed to sp24-01-bjc-teachers February 16, 2024 05:44 Inactive
@ArushC ArushC closed this Feb 17, 2024
@ArushC ArushC deleted the reason-to-denial-reason branch February 17, 2024 01:28
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants