feat: Add endpoint for creating a new reference with or without PDF#549
Conversation
Codecov Report
@@ Coverage Diff @@
## main #549 +/- ##
==========================================
+ Coverage 80.96% 81.07% +0.11%
==========================================
Files 221 222 +1
Lines 13052 13124 +72
Branches 1242 1242
==========================================
+ Hits 10567 10640 +73
+ Misses 2470 2469 -1
Partials 15 15
... and 1 file with indirect coverage changes 📣 We’re building smart automated test selection to slash your CI/CD build times. Learn more |
…-from-s2-search-results
|
Can we add to the to-do list for this pull to return the id of the newly created reference to facilitate removal on the S2 Search dialog (only right after addition ie user made a mistake). |
| reference = create_reference( | ||
| project_id, metadata=request.metadata, url=request.url | ||
| ) | ||
| response = IngestResponse(project_name=project_id, references=[reference]) | ||
|
|
||
| return response |
There was a problem hiding this comment.
Can we add to the to-do list for this pull to return the id of the newly created reference to facilitate removal on the S2 Search dialog (only right after addition ie user made a mistake).
@jvoytek This should be what already happens here. The endpoint returns an IngestResponse with a references key, which in this case would only contain the newly created reference.
|
@gjreda can we return the Currently the create that value in the client and that won't work with the new optional PDF rule. refstudio/src/api/referencesAPI.ts Lines 59 to 63 in 7985127 CC @jvoytek |
gjreda
changed the title
|
I am so very pleased, references without a PDF work like magic this morning. A few things:
|
|
Any idea why this payload wouldn't save the published_date: |
I added it to search results, but forgot to add it in when creating the Reference.
Should be fixed now. This feature breaks a core assumption we've had thus far -- that ingests are happening via the
|
cguedes
left a comment
cguedes
left a comment
There was a problem hiding this comment.
Approved with a note about the downloaded file name.
| raise HTTPError(f"Unable to fetch {url} (status code: {response.status_code})") | ||
|
|
||
| if not metadata.source_filename: | ||
| metadata.source_filename = f"{metadata.title[:200]}.pdf" |
There was a problem hiding this comment.
Is metadata.title[:200] a way of truncating the title to a max of 200 characters? If so, seems too much. The filename should be pretty opaque for the client code (and server) so I think that would be safer to just use an ID (maybe of the reference) instead of the title, that might also include invalid characters for a url.
There was a problem hiding this comment.
I can see the point that 200 might be too long, so I'll shorten this to 75 for now.
I had the thought of showing the ID, but since we show the filename in the file browser on the left side, I thought seeing a random UUID would be a confusing user experience.
We also can't rely on the filename from the URL, since it's often a random ID or even sometimes just pdf.
IMO it'd be best if we asked a user to provide a filename when adding the reference from search results. I liken this to when you're adding a file to VS Code or downloading a PDF to your local -- you need to provide a name for it. I think a similar principle applies when adding a new reference from a URL. Interaction with the reference in the left pane is going to require knowing its name, so I think the user should provide it.
gjreda
deleted the
548-add-endpoints-for-adding-a-new-reference-from-s2-search-results
branch
3 participants
fixes #548
This PR adds a new endpoint to create a reference from some metadata (e.g. from S2 search results). If the reference metadata includes a PDF URL, the PDF will be fetched and added to the project uploads.
TODO:
source_filenameto{title}.pdfAbility to parse Author in a more robust way?Rather than trying to parse on our own, we should probably just keep asfull_name(which sometimes already happens with grobid) and allow the user to edit it in the UI if they'd like to change it.