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

Description Bug when Approving an Allocation Request #455

Merged
merged 5 commits into from
Nov 23, 2022

Conversation

aestoltm
Copy link
Collaborator

@aestoltm aestoltm commented Sep 21, 2022

Fixes #433 : Fixed description not displaying when setting value before clicking 'Approve' for an allocation request. Also Fixes #470

Tested fix:

  1. Approved allocation without any information to ensure that start_date and end_date are automatically applied
  2. Approved allocation without any information to ensure that start_date and end_date are automatically applied along with description text
  3. Approved allocation with custom start_date and end_date along with description text
  4. Approved allocation with custom start_date or end_date along with description text (should have start_date and end_date automatically applied in this case)

@aestoltm aestoltm changed the title Allocation Description Bug when Approving Description Bug when Approving an Allocation Request Sep 21, 2022
@aebruno aebruno requested a review from dsajdak November 10, 2022 13:06
Copy link
Contributor

@dsajdak dsajdak left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This solves the issue of the text being wiped out when hitting the approve button but the bug is also there when clicking the 'deny' button. Interestingly, your testing brings up another issue. There is no way to set a custom start and/or end date when clicking the 'approve' button. If dates are entered in either or both of those boxes, they're wiped out and replaced by start date of today and end date of one year from today. We should have logic built in that sets the dates like that only if dates are not already selected. If there's a start date entered and no end date, the end date should be set one year from the start date (or whatever is set in the coldfront.env config file). This should be a separate git issue though so I will create that (see: #470). However, the issue with the text in the description box being wiped out when clicking the deny button should be fixed as part of this PR, please.

Copy link
Contributor

@dsajdak dsajdak left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Just one minor change requested on this - button got changed to green, I think accidentally. The functionality works as desired though.

@@ -164,7 +164,7 @@ <h3><i class="fas fa-list" aria-hidden="true"></i> Allocation Information</h3>
<div class="float-right">
{% if allocation.status.name == 'New' or allocation.status.name == 'Renewal Requested' %}
<button name="approved" class="btn btn-success mr-1 confirm-activate">Approve</button>
<a href="{% url 'allocation-deny-request' allocation.pk %}" class="btn btn-danger mr-1 confirm-deny">Deny</a>
<button name="denied" class="btn btn-success mr-1 confirm-deny">Deny</button>
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

changing this to btn-success changed the color of the button from red to green

Copy link
Contributor

@dsajdak dsajdak left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM!

@aebruno aebruno merged commit 68ba4b6 into ubccr:master Nov 23, 2022
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.

Allocation: Approve button resets date selection Allocation: Approve button deletes description text
3 participants