added button for resubmitting forms to banner #427
Conversation
|
|
||
| # Add to Banner only if the form is approved | ||
| if form_history.status.statusName == "Approved": | ||
| history_type_data = FormHistory.get(FormHistory.formID == int(form_id)) |
There was a problem hiding this comment.
history_type_data not needed you already declared it above at form_history
|
looks good |
| history_type = str(form_history.historyType) | ||
| labor_form = FormHistory.get(FormHistory.formID == int(form_id), FormHistory.historyType == history_type) | ||
|
|
||
| conn = Banner() |
There was a problem hiding this comment.
what does conn stand for? could you rename it so that it correlates to the issue
There was a problem hiding this comment.
conn stands for Bonner connection, it is used in another function, wanted to keep it consistent
| studentHistoryModalClose(); | ||
| } else { | ||
| msgFlash("Form failed to submit to banner", "fail"); | ||
| studentHistoryModalClose(); |
There was a problem hiding this comment.
Consider leaving the modal open to accept changes when the ajax fails. when you close it when it fails, it requires the user to open the modal again. Since there are three different fail conditions, it might be useful to have a more descriptive fail reason for the user.
There was a problem hiding this comment.
If the modal is not closed after failure, the flash message saying that if failed to submit won't appear.
|
It looks good to me. My only suggestion would be to discuss the name of the variable "conn" with Anderson or Brian. Sometimes, it occurs that a variable name might not be the most preferential, even though it has been previously used. |
|
In addition, be more descriptive in what goals this PR fulfills and the step-by-step process for testing. |
fixes issue #419
Did:
Submit to Bannerbutton on the student history modal in supervisor portal for resubmitting a form to banner that was not properly submitted when the form was approvedTest:
from-backupin lsfApprovedunder Limit Form StatusSubmit to Banner