Add request more info feature as an admin

[perryzjc]

Pivotal Tracker Link

This is basically like 3 stories in one branch, so... yeah. But here is the original story:

Other pivotal tracker links:
Pivotal Tracker Link2
Pivotal Tracker Link3

What this PR does:

Summary of relevant changes:

1. Added a request more info button for admin
2. Added rspec test and cucumber test for this new feature
3. Added a request more info email template
4. Added a new field for liquid engine: request_reason
5. Fix relevant naming for reason

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

Include screenshots, videos, etc.

Who authored this PR?

1. Perry (Jingchao) Zhong
2. Razvan Turcu (Raz)
3. Michael Tao

How should this PR be tested?

• What do the specs/features test?
  • Test email content correctness
  • Test user workflow of requesting more info as an admin
• Are there edge cases to watch out for?
  No.

Are there any complications to deploying this?

Yes. We changed the request info email template, and it's a seed data. To see the change, we need to do rails db:seed

Checklist:

• 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

All modified and coverable lines are covered by tests ✅

Project coverage is 83.75%. Comparing base (092cce2) to head (21718e5).
Report is 7 commits behind head on main.

Additional details and impacted files

@@            Coverage Diff             @@
##             main      #32      +/-   ##
==========================================
+ Coverage   81.95%   83.75%   +1.79%     
==========================================
  Files          20       20              
  Lines         654      677      +23     
==========================================
+ Hits          536      567      +31     
+ Misses        118      110       -8     

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

[ArushC]

This looks good overall to me. Everything appears to function as expected based on the tests. I have a couple of comments. Some of them are stuff I do not understand and would like clarification for in the future, and one is something that I think could be changed in the future:

1. There were two Cucumber step definitions that might need to be turned into English for nontechnical customers:

"And I press "❌" within "#DataTables_Table_0 > tbody > tr:nth-child(1)""
And I press "❓" within "#DataTables_Table_0 > tbody > tr:nth-child(1)"

2. I noticed that in one location in the code, you guys changed the "denial_reason" tags to "action_reason". I assume that you had a good reason for doing so, but I'm not exactly sure why. Is it supposed to be a generic to indicate that the corresponding element could either process a denial or a request more info request?

3. In the Javascript, I saw a reference to the "fallback option". Is there a reason why "denial_reason" is the "fallback option" rather than "request_reason"? And is it even necessary to have a fallback option?

[ArushC]

@perryzjc @razztech @realmichaeltao I would also suggest a validation that ensures the reason field is not blank, but we can ask Michael about this tomorrow. Currently it allows sending blank emails without any reason specified.

[perryzjc]

This looks good overall to me. Everything appears to function as expected based on the tests. I have a couple of comments. Some of them are stuff I do not understand and would like clarification for in the future, and one is something that I think could be changed in the future:

1. There were two Cucumber step definitions that might need to be turned into English for nontechnical customers:

"And I press "❌" within "#DataTables_Table_0 > tbody > tr:nth-child(1)"" And I press "❓" within "#DataTables_Table_0 > tbody > tr:nth-child(1)"

2. I noticed that in one location in the code, you guys changed the "denial_reason" tags to "action_reason". I assume that you had a good reason for doing so, but I'm not exactly sure why. Is it supposed to be a generic to indicate that the corresponding element could either process a denial or a request more info request?
3. In the Javascript, I saw a reference to the "fallback option". Is there a reason why "denial_reason" is the "fallback option" rather than "request_reason"? And is it even necessary to have a fallback option?

@ArushC

1. Just saw your comments. I agree that these two cucumber tests can be converted into English for non-technical customers. We were merely following the code convention (❌) from the previous semester. We can consult with Michael to determine if it's worth improving.

2. Yeah, your guess is correct. At line 69 of that file, modal.find('#action_reason').attr('name', inputName); is used to replace action_reason with the corresponding reason, as that reason name is relevant to the route redirected to.

3. I have the same question about this. We were just reusing the same code (fallback of denial) from the previous semester without additional changes. Maybe we can remove it.

4. Regarding input validation, I am working on it. A new PR will be submitted.