Skip to content

Add more info to cart - #24

Open
vincerubinetti wants to merge 6 commits into
mainfrom
fuller-cart-export
Open

Add more info to cart#24
vincerubinetti wants to merge 6 commits into
mainfrom
fuller-cart-export

Conversation

@vincerubinetti

Copy link
Copy Markdown
Collaborator

So far, this PR just adds tracking of the raw search and term selection to when a study is added to the cart, and passes that info along wholesale to the share cart and download cart endpoints. If you want to get rid of the download cart endpoint, that would be fine with me. It would be easy to do in the frontend: I already have access to all the full cart data I would need (from the batchStudyLookup), and it would allow me to add more download customization more easily in the future (e.g. custom column select).

@netlify

netlify Bot commented Jul 30, 2026

Copy link
Copy Markdown

Deploy Preview for m2on ready!

Name Link
🔨 Latest commit 4d7ff1a
🔍 Latest deploy log https://app.netlify.com/projects/m2on/deploys/6a6bc334cff808000827e8dd
😎 Deploy Preview https://deploy-preview-24--m2on.netlify.app
📱 Preview on mobile
Toggle QR Code...

QR Code

Use your smartphone camera to open QR code link.

To edit notification comments on pull requests, go to your Netlify project configuration.

@falquaddoomi

falquaddoomi commented Jul 30, 2026

Copy link
Copy Markdown
Collaborator

Hey @vincerubinetti, not sure if you're still making changes, but when I test the branch locally I'm seeing just headers and no rows in the exported CSV file and just {"studies":[]} in the JSON export.

Here's my current cart:
Screenshot 2026-07-30 at 12 06 31 PM

@vincerubinetti

Copy link
Copy Markdown
Collaborator Author

The actual assembling is done in the backend, see downloadCart:

const url = new URL(`${api}/cart/download/`);
  url.searchParams.set("type", type);
  url.searchParams.set("filename", filename);
  const options = {
    method: "POST",
    headers: { "Content-Type": "application/json" },
    body: { localCart },
    parse: "blob",
  } as const;
  analytics.event("download_cart", { localCart, filename, type });
  const data = await request(url, z.instanceof(Blob), options);
  if (type === "csv") downloadBlob(data, filename, "csv");
  if (type === "json") downloadBlob(data, filename, "json");
};

The corresponding code needs to be updated in the backend. Unless you want me to just do this in the frontend, which I think is appropriate.

@falquaddoomi

Copy link
Copy Markdown
Collaborator

@vincerubinetti I see; right, forgot about that. It seems the frontend is sending the cart's contents under a different key that the backend wasn't expecting, which I guess lead to the empty cart. The payload also appears to be different; previously IIRC it was just a list of GEO Series IDs under ids, but now there are more fields. Happy to modify the backend to store them, too, if it's ok with you for me to push changes to this PR branch.

Anyway, if you think of carts as a purely front-end thing, then sure, feel free to create downloadable files from them in the frontend. I presume you'll have to push carts to the server at some point to save/share them, unless you intend to serialize them all into the URL.

@vincerubinetti

Copy link
Copy Markdown
Collaborator Author

Yeah, note that some fields got added and changed.

I feel like the downloading part makes sense to just do in the frontend. It feels more like a frontend concern; pretty-downloading of the data in the cart. Otherwise, a user could just use the API to make a cart query to get basically the same JSON data. If you're okay with removing that download endpoint from the backend, I can quickly address @phicks22 's request to add some columns to the downloaded CSV.

The sharing will have to stay in the backend. I think we may have talked about just URL encoding everything needed to share a cart... I guess all we'd need is a list of study ids, a name, some dates, etc. I haven't tested e.g. base64 encoding and/or compressing parameters to fit in a URL. But I imagine it could still potentially end up just being too big for a sharable URL.

@falquaddoomi

Copy link
Copy Markdown
Collaborator

Sure, I can remove the download endpoint and you can just create them in the frontend; I agree that you're basically just sending the data to the backend that you'd use to create the CSV/JSON file anyway.

The sharing will have to stay in the backend. I think we may have talked about just URL encoding everything needed to share a cart... I guess all we'd need is a list of study ids, a name, some dates, etc. I haven't tested e.g. base64 encoding and/or compressing parameters to fit in a URL. But I imagine it could still potentially end up just being too big for a sharable URL.

Frankly I'd prefer us to continue persisting cart items to the backend, unless there's a good reason not to. I can imagine that the URLs could get very long for carts with many items, and who knows how many items people are going to add.

@falquaddoomi

Copy link
Copy Markdown
Collaborator

@vincerubinetti FYI, I've removed the /api/cart/download/ endpoint as you requested. You'll have to reimplement it in the frontend, of course.

@vincerubinetti
vincerubinetti requested a review from phicks22 July 30, 2026 21:34
@vincerubinetti

Copy link
Copy Markdown
Collaborator Author

@falquaddoomi @phicks22 I moved the implementation of the cart download to the frontend. Thus, you can test out the new functionality on the netlify preview. Please let me know if you'd like to change the format of the downloads at all.

@falquaddoomi falquaddoomi 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 to me!

I think it's fine as-is, but a minor improvement suggestion: I see that the database external IDs are present in the JSON, but not in the CSV/TSV downloads. Assuming @phicks22 and @ChristopherMancuso think it'd be useful, perhaps we could create additional columns for each database in the tabular outputs for each database and put the ID, if present, there?

For databases that don't have an ID but show up under databases, perhaps a checkmark or other indicator in the column could suffice.

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