send JSON object in body instead of uploading edited recipe#174
send JSON object in body instead of uploading edited recipe#174
Conversation
|
ascibisz
force-pushed
the
feature/pass-json-recipe
branch
from
00f3d24 to
38be27f
Compare
| RETENTION_PERIODS: { | ||
| RECIPES_EDITED: 24 * 60 * 60 * 1000, // 24 hours | ||
| JOB_STATUS: 24 * 60 * 60 * 1000, // 24 hours | ||
| JOB_STATUS: 30 * 24 * 60 * 60 * 1000, // 30 days |
There was a problem hiding this comment.
since we're re-using results from previous job runs now using the job_status collection, we want to increase it's retention policy
There was a problem hiding this comment.
Pull request overview
This PR removes the step of uploading edited recipes to Firebase, instead sending the recipe JSON directly in the POST request body to the server. It also cleans up unused gradients from the recipe before sending, and extends the JOB_STATUS retention policy to support re-use of previous job runs.
Changes:
- Send edited recipe JSON in POST body instead of uploading to Firebase; remove
addRecipefunction - Strip unreferenced gradients from recipe in
recipeToStringbefore sending to server - Extend JOB_STATUS retention from 24 hours to 30 days and update timestamp after reading final status
Reviewed changes
Copilot reviewed 9 out of 9 changed files in this pull request and generated 3 comments.
Show a summary per file
| File | Description |
|---|---|
| src/App.tsx | Remove Firebase upload flow; send recipe in POST body; update job status timestamp after completion |
| src/utils/recipeLoader.ts | Rename jsonToString to recipeToString with gradient cleanup logic |
| src/utils/firebase.ts | Remove addRecipe, add updateJobStatusTimestamp, replace setDoc with updateDoc |
| src/constants/aws.ts | Make recipe param optional in getSubmitPackingUrl |
| src/constants/firebase.ts | Extend JOB_STATUS retention to 30 days |
| src/components/InputSwitch/index.tsx | Reduce numeric display precision from 4 to 2 decimal places |
| src/test/recipeLoader.test.ts | Update to use renamed recipeToString |
| src/test/test-files/ER_peroxisome.json | Remove unreferenced gradients from test fixture |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
| if (typeof value == "number") { | ||
| value = value * conversion; | ||
| value = Number(value.toFixed(4)); | ||
| value = Number(value.toFixed(2)); |
There was a problem hiding this comment.
Previously, we were keeping 4 digits after the decimal point and it was getting cut off in the box on the form. It feels like 2 digits after the decimal point is sufficient?
* set retention for job status to 30 days, update job status timestamp on read, and misc bug fixes * code cleanup
* remove irrelevant gradient data from final recipe string * round radius to 2 decimal points * update test file to remove unnecessary gradients * copilot suggestions
Co-authored-by: Copilot <175728472+Copilot@users.noreply.github.com>
ascibisz
force-pushed
the
feature/pass-json-recipe
branch
from
b2cfeb8 to
f960773
Compare
| const firebaseRecipe = recipeChanged | ||
| ? undefined | ||
| : "firebase:recipes/" + recipeId; | ||
| const firebaseConfig = configId | ||
| ? "firebase:configs/" + configId | ||
| : undefined; |
There was a problem hiding this comment.
double check if this is still necessary
interim17
left a comment
interim17
left a comment
There was a problem hiding this comment.
Consider encoding the url so it handle special characters, otherwise LGTM
| let url = SUBMIT_PACKING_ECS; | ||
| if (recipe && config) { | ||
| url += `?recipe=${recipe}&config=${config}`; | ||
| } else if (recipe) { | ||
| url += `?recipe=${recipe}`; | ||
| } else if (config) { | ||
| url += `?config=${config}`; |
There was a problem hiding this comment.
Safer to encode with this API as long as the result works on the backend: https://developer.mozilla.org/en-US/docs/Web/API/URLSearchParams/URLSearchParams
| const requestBody = recipeChanged ? recipeString : undefined; | ||
| const request: RequestInfo = new Request(url, { method: "POST", body: requestBody }); |
There was a problem hiding this comment.
Content-Type: application/json?
Problem
In an effort to improve the efficiency of our packings, we want to remove the step where we're posting the edited recipe to firebase from the client and instead send the edited recipe JSON to the server in the body of the POST request, and then the server will handle any necessary firebase interactions.
Part of this ticket
Note: Deploy PR with server side changes before merging this PR.
Solution