Skip to content

Conversation

simonLeary42
Copy link
Contributor

@simonLeary42 simonLeary42 commented Oct 3, 2025

The list of allocation statuses is long and rather confusing. Coldfront's behavior with respect to these statuses is inconsistent. It seems as though the list of statuses has grown over time, and some statuses were added without also being accounted for in various filters and conditionals. This isn't surprising because accounting for all statuses in all filters and conditionals is a lot of work. I think many of these statuses would be more appropriate as tags.

Here are some rules I came up with on my own about how I think statuses were meant to be used (please correct me if I'm wrong):

  • statuses used before and only before an allocation is active:
    • New
    • Denied
    • Approved
    • Pending
    • Payment Requested
    • Payment Pending
    • Unpaid
    • Paid
  • statuses used while and only while an allocation is active:
    • Active
    • Renewal Requested
  • statuses used after and only after an allocation is active:
    • Expired
    • Revoked

Assuming the above rules are followed, Coldfront's behavior is still surprising in many situations:

  • If an allocation changes from status Active to status Expired, the allocation_disable and allocation_remove_user signals will not be triggered, so plugins cannot respond. These signals trigger only when an allocation changes to statuses Denied or Revoked.
  • If a PI removes a user from their project, that user will be removed from all their project's allocations, except allocations with status Expired or Revoked.
  • Allocations with statuses New, Paid, Payment Pending, and Payment Requested count towards the allocation limit, but allocations with statuses Approved, Payment Declined, Pending, and Unpaid do not.
  • The start date cannot be removed from an allocation with status Active, but it can be removed from an allocation with status Renewal Requested.
  • The end date cannot be removed from an allocation with status Expired, but it can be removed from an allocation with status Revoked.

This PR does not change this behavior in any way, instead it organizes the behavior for all situations in one file, so it can be easily understood. This PR also makes this behavior configurable, which is a nice bonus.

The tests all pass. For reference, here are the tests that fail if I make all the allocation status lists into empty lists:

Creating test database for alias 'default'...
System check identified no issues (0 silenced).
FFF..FFF................F......FFF............F.............................................................s..............sssss...
======================================================================
FAIL: test_status_is_active_and_no_end_date_has_validation_error (coldfront.core.allocation.tests.test_models.AllocationModelCleanMethodTests.test_status_is_active_and_no_end_date_has_validation_error)
Test that an allocation with status 'active' and no end date raises a validation error.
----------------------------------------------------------------------
Traceback (most recent call last):
  File "/srv/simon/src/coldfront/coldfront/core/allocation/tests/test_models.py", line 114, in test_status_is_active_and_no_end_date_has_validation_error
    with self.assertRaises(ValidationError):
AssertionError: ValidationError not raised

======================================================================
FAIL: test_status_is_active_and_no_start_date_has_validation_error (coldfront.core.allocation.tests.test_models.AllocationModelCleanMethodTests.test_status_is_active_and_no_start_date_has_validation_error)
Test that an allocation with status 'active' and no start date raises a validation error.
----------------------------------------------------------------------
Traceback (most recent call last):
  File "/srv/simon/src/coldfront/coldfront/core/allocation/tests/test_models.py", line 106, in test_status_is_active_and_no_start_date_has_validation_error
    with self.assertRaises(ValidationError):
AssertionError: ValidationError not raised

======================================================================
FAIL: test_status_is_active_and_start_date_after_end_date_has_validation_error (coldfront.core.allocation.tests.test_models.AllocationModelCleanMethodTests.test_status_is_active_and_start_date_after_end_date_has_validation_error)
Test that an allocation with status 'active' and start date after end date raises a validation error.
----------------------------------------------------------------------
Traceback (most recent call last):
  File "/srv/simon/src/coldfront/coldfront/core/allocation/tests/test_models.py", line 125, in test_status_is_active_and_start_date_after_end_date_has_validation_error
    with self.assertRaises(ValidationError):
AssertionError: ValidationError not raised

======================================================================
FAIL: test_status_is_expired_and_end_date_not_past_has_validation_error (coldfront.core.allocation.tests.test_models.AllocationModelCleanMethodTests.test_status_is_expired_and_end_date_not_past_has_validation_error)
Test that an allocation with status 'expired' and end date in the future raises a validation error.
----------------------------------------------------------------------
Traceback (most recent call last):
  File "/srv/simon/src/coldfront/coldfront/core/allocation/tests/test_models.py", line 68, in test_status_is_expired_and_end_date_not_past_has_validation_error
    with self.assertRaises(ValidationError):
AssertionError: ValidationError not raised

======================================================================
FAIL: test_status_is_expired_and_no_end_date_has_validation_error (coldfront.core.allocation.tests.test_models.AllocationModelCleanMethodTests.test_status_is_expired_and_no_end_date_has_validation_error)
Test that an allocation with status 'expired' and no end date raises a validation error.
----------------------------------------------------------------------
Traceback (most recent call last):
  File "/srv/simon/src/coldfront/coldfront/core/allocation/tests/test_models.py", line 59, in test_status_is_expired_and_no_end_date_has_validation_error
    with self.assertRaises(ValidationError):
AssertionError: ValidationError not raised

======================================================================
FAIL: test_status_is_expired_and_start_date_after_end_date_has_validation_error (coldfront.core.allocation.tests.test_models.AllocationModelCleanMethodTests.test_status_is_expired_and_start_date_after_end_date_has_validation_error)
Test that an allocation with status 'expired' and start date after end date raises a validation error.
----------------------------------------------------------------------
Traceback (most recent call last):
  File "/srv/simon/src/coldfront/coldfront/core/allocation/tests/test_models.py", line 79, in test_status_is_expired_and_start_date_after_end_date_has_validation_error
    with self.assertRaises(ValidationError):
AssertionError: ValidationError not raised

======================================================================
FAIL: test_allocationaddusersview_access (coldfront.core.allocation.tests.test_views.AllocationAddUsersViewTest.test_allocationaddusersview_access)
Test access to AllocationAddUsersView
----------------------------------------------------------------------
Traceback (most recent call last):
  File "/srv/simon/src/coldfront/coldfront/core/allocation/tests/test_views.py", line 332, in test_allocationaddusersview_access
    self.allocation_access_tstbase(self.url)
  File "/srv/simon/src/coldfront/coldfront/core/allocation/tests/test_views.py", line 69, in allocation_access_tstbase
    utils.test_logged_out_redirect_to_login(self, url)
  File "/srv/simon/src/coldfront/coldfront/core/test_helpers/utils.py", line 40, in test_logged_out_redirect_to_login
    test_case.assertRedirects(response, f"/user/login?next={page}")
  File "/srv/simon/src/coldfront/.venv/lib/python3.12/site-packages/django/test/testcases.py", line 555, in assertRedirects
    self.assertEqual(
AssertionError: 302 != 200 : Couldn't retrieve redirection page '/allocation/1/': response code was 302 (expected 200)

======================================================================
FAIL: test_allocationchangeview_access (coldfront.core.allocation.tests.test_views.AllocationChangeViewTest.test_allocationchangeview_access)
Test get request
----------------------------------------------------------------------
Traceback (most recent call last):
  File "/srv/simon/src/coldfront/coldfront/core/allocation/tests/test_views.py", line 172, in test_allocationchangeview_access
    self.allocation_access_tstbase(self.url)
  File "/srv/simon/src/coldfront/coldfront/core/allocation/tests/test_views.py", line 69, in allocation_access_tstbase
    utils.test_logged_out_redirect_to_login(self, url)
  File "/srv/simon/src/coldfront/coldfront/core/test_helpers/utils.py", line 40, in test_logged_out_redirect_to_login
    test_case.assertRedirects(response, f"/user/login?next={page}")
  File "/srv/simon/src/coldfront/.venv/lib/python3.12/site-packages/django/test/testcases.py", line 555, in assertRedirects
    self.assertEqual(
AssertionError: 302 != 200 : Couldn't retrieve redirection page '/allocation/1/': response code was 302 (expected 200)

======================================================================
FAIL: test_allocationchangeview_post_extension (coldfront.core.allocation.tests.test_views.AllocationChangeViewTest.test_allocationchangeview_post_extension)
Test post request to extend end date
----------------------------------------------------------------------
Traceback (most recent call last):
  File "/srv/simon/src/coldfront/coldfront/core/allocation/tests/test_views.py", line 183, in test_allocationchangeview_post_extension
    self.assertContains(response, "Allocation change request successfully submitted.")
  File "/srv/simon/src/coldfront/.venv/lib/python3.12/site-packages/django/test/testcases.py", line 660, in assertContains
    self.assertTrue(
AssertionError: False is not true : Couldn't find 'Allocation change request successfully submitted.' in response

======================================================================
FAIL: test_allocationchangeview_post_no_change (coldfront.core.allocation.tests.test_views.AllocationChangeViewTest.test_allocationchangeview_post_no_change)
Post request with no change should not go through
----------------------------------------------------------------------
Traceback (most recent call last):
  File "/srv/simon/src/coldfront/coldfront/core/allocation/tests/test_views.py", line 193, in test_allocationchangeview_post_no_change
    self.assertContains(response, "You must request a change")
  File "/srv/simon/src/coldfront/.venv/lib/python3.12/site-packages/django/test/testcases.py", line 660, in assertContains
    self.assertTrue(
AssertionError: False is not true : Couldn't find 'You must request a change' in response

======================================================================
FAIL: test_allocationremoveusersview_access (coldfront.core.allocation.tests.test_views.AllocationRemoveUsersViewTest.test_allocationremoveusersview_access)
----------------------------------------------------------------------
Traceback (most recent call last):
  File "/srv/simon/src/coldfront/coldfront/core/allocation/tests/test_views.py", line 355, in test_allocationremoveusersview_access
    self.allocation_access_tstbase(self.url)
  File "/srv/simon/src/coldfront/coldfront/core/allocation/tests/test_views.py", line 69, in allocation_access_tstbase
    utils.test_logged_out_redirect_to_login(self, url)
  File "/srv/simon/src/coldfront/coldfront/core/test_helpers/utils.py", line 40, in test_logged_out_redirect_to_login
    test_case.assertRedirects(response, f"/user/login?next={page}")
  File "/srv/simon/src/coldfront/.venv/lib/python3.12/site-packages/django/test/testcases.py", line 555, in assertRedirects
    self.assertEqual(
AssertionError: 302 != 200 : Couldn't retrieve redirection page '/allocation/1/': response code was 302 (expected 200)

----------------------------------------------------------------------
Ran 131 tests in 3.297s

FAILED (failures=11, skipped=6)
Destroying test database for alias 'default'...

Future work:

  • a few FIXME comments have been added, these should be fixed as soon as this PR goes through
    • I would just fix them in this PR, but I want the scope of this PR to be limited to purely refactoring, no behavior changes
  • many of these status lists should be combined and simplified
    • I would personally like to see the status list reduced to just New, Active, Inactive, with all other statuses becoming tags
  • some of the above behavior should probably be changed
  • if I am right about how these statuses were meant to be used, I would like to add it to the documentation, and I would like to see enforcement that they are used this way
  • ALLOCATION_STATUSES_UNSET_START_DATE_END_DATE and ALLOCATION_STATUSES_DO_DISABLE should be separate conditionals, rather than having one nested in the other

@simonLeary42 simonLeary42 force-pushed the refactor-allocation-state-lists branch from 59fc17d to ced9c33 Compare October 3, 2025 22:19
@simonLeary42 simonLeary42 mentioned this pull request Oct 6, 2025
@simonLeary42 simonLeary42 force-pushed the refactor-allocation-state-lists branch 3 times, most recently from 797f67f to 441af5f Compare October 6, 2025 14:29
@simonLeary42 simonLeary42 force-pushed the refactor-allocation-state-lists branch 14 times, most recently from eafb7b0 to fc002b2 Compare October 7, 2025 17:57
@simonLeary42 simonLeary42 marked this pull request as ready for review October 7, 2025 18:01
@simonLeary42 simonLeary42 force-pushed the refactor-allocation-state-lists branch 2 times, most recently from e1c65aa to 38bed79 Compare October 10, 2025 12:24
@simonLeary42 simonLeary42 changed the title refactor allocation state lists refactor allocation status lists Oct 10, 2025
Signed-off-by: Simon Leary <simon.leary42@proton.me>
@simonLeary42 simonLeary42 force-pushed the refactor-allocation-state-lists branch from 38bed79 to 079d3d7 Compare October 10, 2025 20:14
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.

1 participant