Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

fix: search api will not set magic date header when requesting /-/all without ?since #1598

Merged
merged 3 commits into from
Dec 14, 2019

Conversation

favoyang
Copy link
Contributor

Type:

The following has been addressed in the PR:

Description:
Fix #1597, only set magic date header when requesting /-/all without ?since

@favoyang favoyang changed the title fix: search api will not set magic date header when requesting /-/all without ?since fix: search api will not set magic date header when requesting /-/all without ?since Nov 28, 2019
@codecov
Copy link

codecov bot commented Nov 28, 2019

Codecov Report

Merging #1598 into master will not change coverage.
The diff coverage is 100%.

@@           Coverage Diff           @@
##           master    #1598   +/-   ##
=======================================
  Coverage   84.88%   84.88%           
=======================================
  Files          47       47           
  Lines        2475     2475           
  Branches      570      579    +9     
=======================================
  Hits         2101     2101           
+ Misses        370      369    -1     
- Partials        4        5    +1
Impacted Files Coverage Δ
src/api/endpoint/api/search.ts 83.72% <100%> (ø) ⬆️
src/api/web/endpoint/package.ts 74.13% <0%> (ø) ⬆️

@juanpicado
Copy link
Member

I've been looking into this for a while, I think the current search is quite old, pretty much the same as sinopia. My goal is make this #310 and forget about this endpoint.

I was also looking into the https://github.com/npm/cli/blob/latest/test/tap/all-package-metadata.js and the current implementation that haven't change much after PR npm/npm@2b8057b where the _updated field was introduced.

Anyway, after run some test I think many workaround here does not make much sense, but also I could not reproduce perfectly #1597 . I'll merge and see how it goes, not much people is using npm search as far I know.

Thanks for look into this endpoint, hopefully we can get rid of it and use #310 instead.

@juanpicado juanpicado merged commit 158de3f into verdaccio:master Dec 14, 2019
@favoyang
Copy link
Contributor Author

@juanpicado thanks for the merge, and sure #310 is more preferred.

@lock
Copy link

lock bot commented Dec 24, 2019

🤖This thread has been automatically locked 🔒 since there has not been any recent activity after it was closed.
We lock tickets after 90 days with the idea to encourage you to open a ticket with new fresh data and to provide you better feedback 🤝and better visibility 👀.
If you consider, you can attach this ticket 📨 to the new one as a reference for better context.
Thanks for being a part of the Verdaccio community! 💘

@lock lock bot locked as resolved and limited conversation to collaborators Dec 24, 2019
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Bug: search api shall only set magic date header when request /-/all without ?since
2 participants