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

survey association lock removal #318

Merged
merged 7 commits into from
Oct 2, 2024

Conversation

ayobi
Copy link
Contributor

@ayobi ayobi commented Apr 17, 2024

Removed post_claim_samples call to avoid survey to sample lock on the front end as it will be done on the backend before the sample metadata is pushed to qiita.

@ayobi ayobi marked this pull request as ready for review April 18, 2024 21:10
@@ -2768,14 +2769,6 @@ def post_claim_samples(*, account_id=None, source_id=None, body=None,
if has_error:
return survey_output

# TODO: this will have to get more nuanced when we add animal surveys?
Copy link
Collaborator

Choose a reason for hiding this comment

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

I think there are still some pieces that can come out of this function, e.g. the survey_output section directly above (GitHub's UI won't let me contextually comment on lines that aren't being changed in the PR). Also, the comment above the function definition is no longer applicable.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Ah ok, so all we're left here now is just the reconsent if needed and claiming of a sample

for curr_sample_id in sample_ids_to_claim:
# Claim sample
has_error, sample_output, _ = ApiRequest.post(
'/accounts/{0}/sources/{1}/samples'.format(
account_id, source_id),
json={"sample_id": curr_sample_id})
if has_error:
return sample_output

@cassidysymons cassidysymons merged commit d8ff23c into biocore:master Oct 2, 2024
3 checks passed
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