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

[IAC][ENG-6998] Infinite Resubmits For IAC #10937

Open
wants to merge 2 commits into
base: develop
Choose a base branch
from

Conversation

Johnetordoff
Copy link
Contributor

@Johnetordoff Johnetordoff commented Jan 21, 2025

Purpose

Allows a institutional access user to resubmit node requests. Before curator would get rejected accidently and be banned outside the node without way of making new request, This feature allows admin to update and re-new their request an infinite number of times.

Changes

  • add resubmit behavior and testing

QA Notes

Please make verification statements inspired by your code and what your code touches.

  • Verify
  • Verify

What are the areas of risk?

Any concerns/considerations/questions that development raised?

Documentation

Side Effects

Ticket

https://openscience.atlassian.net/browse/ENG-6998

@Johnetordoff Johnetordoff changed the base branch from develop to feature/institutional_access January 21, 2025 20:24
@Johnetordoff Johnetordoff marked this pull request as ready for review January 21, 2025 20:42
@Johnetordoff Johnetordoff changed the base branch from feature/institutional_access to develop January 21, 2025 20:51
@Johnetordoff Johnetordoff changed the base branch from develop to feature/institutional_access January 21, 2025 20:51
@Johnetordoff Johnetordoff marked this pull request as draft January 21, 2025 21:09
@Johnetordoff Johnetordoff marked this pull request as ready for review January 21, 2025 21:09
@Johnetordoff Johnetordoff changed the base branch from feature/institutional_access to develop January 21, 2025 21:09
@Johnetordoff Johnetordoff changed the base branch from develop to feature/institutional_access January 21, 2025 21:09
@Johnetordoff Johnetordoff changed the base branch from feature/institutional_access to develop January 22, 2025 14:46
@Johnetordoff Johnetordoff force-pushed the infinite-resubmits-for-IAC branch from 18c58cc to 24a4f0d Compare January 22, 2025 14:47
@Johnetordoff Johnetordoff force-pushed the infinite-resubmits-for-IAC branch from 24a4f0d to 92b8388 Compare January 22, 2025 14:48
requested_permissions=requested_permissions,
)
node_request.save()
with transaction.atomic():
Copy link
Contributor Author

Choose a reason for hiding this comment

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

Because the error would happen mid-same, you need this transaction to be able to rollback on error.

Copy link
Collaborator

@brianjgeiger brianjgeiger left a comment

Choose a reason for hiding this comment

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

One typo, and a question: What happens if the request is not accepted or rejected, but they submit multiple requests for the same node? Does the contributor request show up multiple times in the request for access section of the contributors page? This may be too simple of a fix for this, but I'm not super-familiar with this code, so it may be fine.


def test_requester_can_resubmit(self, app, project, institutional_admin, url, create_payload):
"""
Test that a requester cannot submit another access request for the same node.
Copy link
Collaborator

Choose a reason for hiding this comment

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

Suggested change
Test that a requester cannot submit another access request for the same node.
Test that a requester can submit another access request for the same node.

@Johnetordoff
Copy link
Contributor Author

@brianjgeiger Currently a uniqueness constraint ensures their is only ever one request type per requester per node. It is enforced here: https://github.com/CenterForOpenScience/osf.io/blob/develop/osf/migrations/0003_aggregated_runsql_calls.py#L24 it's a raw sql uniqueness constraint. If you look at the code again with that in mind it makes much more sense.

@brianjgeiger
Copy link
Collaborator

Okay, so I would guess that they get an error toast in the front end if they try to resubmit to a node where there is an active request for access?

@Johnetordoff
Copy link
Contributor Author

Johnetordoff commented Jan 23, 2025

@brianjgeiger yes it says: f"Users may not have more than one {request_type} request per node.") it changes so it now when you update the request, it sends a green success toast.

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