Remaining Hacking Together Changes#354
Conversation
|
Azure Static Web Apps: Your stage site is ready! Visit it here: https://witty-hill-08172a210-354.centralus.5.azurestaticapps.net |
|
@codex review |
There was a problem hiding this comment.
Pull request overview
This PR adjusts how SynBioHub overwrite behavior is represented and passed through the frontend upload flows, and adds a backend Python dependency.
Changes:
- Updated the collection browser workflow result to emit new overwrite mode values.
- Updated upload parameter serialization for
sbh_overwriteinupload_resource/uploadExperimentand added debug logging. - Added
packaging==25.0to backend requirements.
Reviewed changes
Copilot reviewed 4 out of 4 changed files in this pull request and generated 3 comments.
| File | Description |
|---|---|
| frontend/src/modules/unified_modal/CollectionBrowserModal.jsx | Changes the overwrite mode value emitted by the collection-selection workflow. |
| frontend/src/components/panels/xdc/CollectionWizard.jsx | Adjusts the arguments passed into uploadExperiment from the collection wizard. |
| frontend/src/API.js | Changes how sbh_overwrite is encoded into the Params payload; adds a console log of Params JSON. |
| backend/requirements.txt | Adds a Python dependency pin (but currently duplicates an existing entry). |
Comments suppressed due to low confidence (1)
frontend/src/API.js:148
- Same issue here: coercing
sbh_overwriteto a boolean may change the overwrite mode value observed by the backend/client library. Prefer passing through the expected numeric overwrite code (or mapping UI values to the correct numeric code) instead oftrue/false.
fj_token: null,
fj_user: null,
fj_pass: null,
collection_url: collectionUrl,
sbh_overwrite: sbh_overwrite == 3 ? true : false,
fj_overwrite: 1,
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
| fj_token: null, | ||
| fj_user: null, | ||
| fj_pass: null, | ||
| collection_url: collectionUrl, | ||
| sbh_overwrite: sbh_overwrite, | ||
| sbh_overwrite: sbh_overwrite == 3 ? true : false, | ||
| fj_overwrite: 1, |
| } | ||
|
|
||
| const paramsJson = JSON.stringify(paramsObj); | ||
| console.log(paramsJson) |
| collections: Array.from(selectedCollections.values()), | ||
| count: selectedCollections.size, | ||
| sbh_overwrite: overwrite ? 2 : 0, | ||
| sbh_overwrite: overwrite ? 3 : 2, |
![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: 89c247508d
ℹ️ 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".
| fj_pass: null, | ||
| collection_url: collectionUrl, | ||
| sbh_overwrite: sbh_overwrite, | ||
| sbh_overwrite: sbh_overwrite == 3 ? true : false, |
There was a problem hiding this comment.
This conversion only treats the numeric value 3 as overwrite-enabled, so existing workflow files created before this commit (which stored overwrite as 2) are now downgraded to false when users reopen and upload again. In practice, previously configured "overwrite" study updates will silently stop overwriting and can fail on duplicate content; the serializer should accept legacy truthy encodings (at least 2, and ideally boolean true) to preserve backward compatibility.
Useful? React with 👍 / 👎.
| } | ||
|
|
||
| const paramsJson = JSON.stringify(paramsObj); | ||
| console.log(paramsJson) |
There was a problem hiding this comment.
Logging paramsJson exposes SynBioHub credentials/tokens in browser console output because this payload includes sbh_token; anyone with local console access (or exported debug logs/screenshots) can recover secrets. This creates an avoidable credential leakage path in normal upload flows and should be removed or explicitly redacted.
Useful? React with 👍 / 👎.
|
DO NOT MERGE -- NOT SURE WHAT THESE CHANGES DO -- JUST SAW THEY EXISTED DURING DEMO WITH NEW SBS TEAM |
No description provided.