feat(chemistry): add well show major and minor chemistry summary cards#234
feat(chemistry): add well show major and minor chemistry summary cards#234
Conversation
…ies and integration into WellShow
jirhiker
changed the title
Preview DeploymentPreview URL: $PREVIEW_URL Note: This preview uses the staging API endpoints and has auth disabled for testing. |
![chatgpt-codex-connector[bot]](https://avatars.githubusercontent.com/in/1144995?s=60&v=4){kind=small}
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 3d2d147e2f
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
| } | ||
|
|
||
| const serverSideNameRows = await getFeatures({ name: wellName as string }) | ||
| if (serverSideNameRows.length > 0) return serverSideNameRows[0] |
There was a problem hiding this comment.
This returns the first server row without any ordering, so the summary can show stale analytes/date when a collection contains multiple records for the same well (especially possible when resolving *_chemistry_results). Because the UI labels this as “Latest Chemistry Date,” choosing rows[0] is not reliable; request explicit descending sort or pick the max latest_chemistry_date/sample_date client-side before normalizing.
Useful? React with 👍 / 👎.
| return (page?.data ?? []) as MajorChemistryFeature[] | ||
| } | ||
|
|
||
| const serverSideNameRows = await getFeatures({ name: wellName as string }) |
There was a problem hiding this comment.
The query filters by name and then immediately uses the first result, but well names are not guaranteed unique and server-side name filtering may be non-exact. In those cases this page can silently display chemistry from another well even though the route already has a stable well id; prefer filtering/validating by thing_id/well_id (or at least verify the returned feature matches the current well) before building the summary.
Useful? React with 👍 / 👎.
I like the null state
|
|
Could you post a screenshot of this feature showing data. Wasn't able to find a good example. |
|
TylerAdamMartinez
left a comment
TylerAdamMartinez
left a comment
There was a problem hiding this comment.
If the summary fields are meant to be changeable/editable, then it makes sense. Otherwise, perhaps those summaries could be typographed in the same style as the other cards.
| } | ||
| } | ||
|
|
||
| const SummaryField = ({ label, value }: { label: string; value: string }) => ( |
There was a problem hiding this comment.
What made you decide to use a read-only text field instead of MUI Typography?
2 participants
Why
This PR addresses the following problem / context:
How
Implementation summary - the following was changed / added / removed:
name=<well.name>and normalized the returned latest feature into read-only summary fields.Notes
Any special considerations, workarounds, or follow-up work to note?