Conversation
… and configurations
- Keep existing field order/labels aligned with legacy model
- Set ThingAdmin.identity to thing for stable linking
- Add HasOne("thing") relationship link in Chemistry Sample Info list/detail
ksmuczynski
requested review from
chasetmartin,
Copilot,
jacob-a-brown,
jirhiker and
kbighorse
Contributor
There was a problem hiding this comment.
Pull request overview
This PR implements a read-only admin view for NMA Chemistry Sample Info with expanded field coverage and improved navigation. The changes focus on making the view consistent with legacy field naming and preventing data modification through the admin interface.
Changes:
- Made ChemistrySampleInfoAdmin read-only by disabling create, edit, and delete operations
- Expanded visible, sortable, and searchable fields to include legacy columns and collection metadata
- Added Thing relationship link for navigation and set stable identity on ThingAdmin
Reviewed changes
Copilot reviewed 2 out of 2 changed files in this pull request and generated 4 comments.
| File | Description |
|---|---|
| admin/views/thing.py | Added stable identity attribute for relationship linking |
| admin/views/chemistry_sampleinfo.py | Implemented read-only behavior, expanded field coverage, added Thing relationship, and updated labels |
Co-authored-by: Copilot <175728472+Copilot@users.noreply.github.com>
Add missing "nma_" prefix Co-authored-by: Copilot <175728472+Copilot@users.noreply.github.com>
Co-authored-by: Copilot <175728472+Copilot@users.noreply.github.com>
Co-authored-by: Copilot <175728472+Copilot@users.noreply.github.com>
Comment on lines
+50
to
+57
| def can_create(self, request: Request) -> bool: | ||
| return False | ||
|
|
||
| def can_edit(self, request: Request) -> bool: | ||
| return False | ||
|
|
||
| def can_delete(self, request: Request) -> bool: | ||
| return False |
There was a problem hiding this comment.
The Request type is referenced but not imported. Add 'from starlette.requests import Request' or the appropriate request type import for this codebase.
| "nma_object_id", | ||
| "nma_location_id", | ||
| "thing_id", | ||
| HasOne("thing", identity="thing"), |
There was a problem hiding this comment.
The HasOne class is referenced but not imported. Add the appropriate import statement for HasOne at the top of the file.
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
2 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Why
This PR addresses the following problem / context:
How
Implementation summary - the following was changed / added / removed:
Notes
Any special considerations, workarounds, or follow-up work to note?