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 feature #163

Merged
merged 12 commits into from
Apr 22, 2017
Merged

Fix search feature #163

merged 12 commits into from
Apr 22, 2017

Conversation

AvailCat
Copy link
Member

  • Check permission in search API
  • Fix author's name not show in search result
  • Remove unused 'allow_failures' from travis config
  • Fix search not work for multiple storage
  • Fix deprecared warning with fs.unlink
  • Fix npm search not show author's name
  • Fix loading icon not show after first search
  • Fix loading icon not align to center

Pick from fl4re's fork:

  • Allow set intermediate SSL certificate
  • Partial fix of search leak private packages name

I'm not familiar with git and just destroyed my repo and lost some changes... So i deleted my GitHub repo and reopen a new PR here, sorry for this.

@juanpicado

Emmanuel Narh and others added 11 commits April 21, 2017 11:53
The https module allows for an intermediate certificate in the options.
It was somehow missed. Adding it back since I had a certificate that
included an intermediate certificate.
- Check permission in search API
- Fix author's name not show in search result
Since NodeJS 7,  call async api without callback is deprecared.
Added 'access' check to search route
Handle failed Ajax search request as 'No result'
@juanpicado
Copy link
Member

Thanks @Meeeeow I'll check asap , awesome

@@ -133,7 +133,11 @@ module.exports = function(config, auth, storage) {
var getData = function(i) {
storage.get_package(results[i].ref, function(err, entry) {
if (!err && entry) {
packages.push(entry.versions[entry['dist-tags'].latest])
auth.allow_access(entry.name, req.remote_user, function(err, allowed) { // TODO: This may cause performance issue?
Copy link
Member

@juanpicado juanpicado Apr 21, 2017

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Reply of #161 (comment)
@Meeeeow
About the can('access'), yeah you are right. No way to use the middleware. Perhaps you update allowed as is_allowed, to be consistent with the middleware.
About the performance, I haven't tested with thousands of packages, but, it worth to try in the future and see what's happen.

@juanpicado
Copy link
Member

Aside of the small review, LGTM 👍

@juanpicado juanpicado mentioned this pull request Apr 21, 2017
@juanpicado juanpicado merged commit d824821 into verdaccio:master Apr 22, 2017
@juanpicado juanpicado added this to the 2.1.5 milestone Apr 22, 2017
@AvailCat AvailCat deleted the fix_search branch April 22, 2017 09:10
@lock
Copy link

lock bot commented May 31, 2018

This thread has been automatically locked since there has not been any recent activity after it was closed. Please open a new issue for related bugs.

@lock lock bot locked as resolved and limited conversation to collaborators May 31, 2018
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.

None yet

2 participants