Skip to content

add more data for resources#30

Merged
amandazhuyilan merged 8 commits into
mainfrom
AAI-221-add-more-fields-for-resources
Jun 13, 2025
Merged

add more data for resources#30
amandazhuyilan merged 8 commits into
mainfrom
AAI-221-add-more-fields-for-resources

Conversation

@amandazhuyilan

@amandazhuyilan amandazhuyilan commented Jun 12, 2025

Copy link
Copy Markdown
Contributor

Description

AAI-221: Add last_updated and updated_by in Resource models.

Changes

  • Added updated_by field to Resource models.
  • Modified approve_resource to require updated_by.
  • Updated tests to reflect and verify updated_by presence.

Checklist

  • I have commented my code, particularly in hard-to-understand areas
  • I have added unit / integration tests that prove my fix is effective or that my feature works
  • I have run all tests locally and they pass
  • I have updated the documentation (if applicable)

How to Test Manually (if necessary)

All tests shall pass.

@marius-mather marius-mather left a comment

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

Looks good - just one question about whether we should have user emails in the updated_by field or that should only reflect admins/people who are authorized to do approvals.

Comment thread README.md Outdated
Comment thread routers/bpa_register.py Outdated
Comment thread routers/bpa_register.py Outdated

@marius-mather marius-mather left a comment

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

Looks great, just one tiny change to suggest

Comment thread schemas/biocommons.py Outdated

@marius-mather marius-mather left a comment

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

all good! 👍

@amandazhuyilan amandazhuyilan merged commit a22197e into main Jun 13, 2025
1 check 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