-
Notifications
You must be signed in to change notification settings - Fork 218
Conversation
Signed-off-by: Andres Martinez Gotor <andres@bitnami.com>
if params["repo"] != "" { | ||
conditions["repo.name"] = params["repo"] | ||
} | ||
if err := db.C(chartCollection).Find(conditions).All(&charts); err != nil { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
So if there are no matches MongoDB returns an error? Doesn't it just return an empty list with no error?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
it doesn't return an error, it logs that the query didn't return any chart and it returns an empty list
Looks like this PR is based on #605 as it's looking for
|
circle build helm#609
* chartsvc: add support for pagination Signed-off-by: Andres Martinez Gotor <andres@bitnami.com> * Apply review changes. Pre-filter unique charts Signed-off-by: Andres Martinez Gotor <andres@bitnami.com> * Update kubeapps/common dep Signed-off-by: Andres Martinez Gotor <andres@bitnami.com> * Use aggregation to query unique charts. Apply other review comments Signed-off-by: Andres Martinez Gotor <andres@bitnami.com> * Count using the pipeline Signed-off-by: Andres Martinez Gotor <andres@bitnami.com> * Update common dep. Minor review Signed-off-by: Andres Martinez Gotor <andres@bitnami.com>
* Signed-off-by: Himanshu Pandey <hpandey@pivotal.io> Added check for empty charts * Added Test case for Empty Charts Signed-off-by: Himanshu Pandey <hpandey@pivotal.io> * Corrected the test case Signed-off-by: Himanshu Pandey <hpandey@pivotal.io>
Signed-off-by: Andres Martinez Gotor <andres@bitnami.com>
Yes, I have rebased master now that that PR is merged. |
Signed-off-by: Andres Martinez Gotor <andres@bitnami.com>
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
lgtm, thanks!
Signed-off-by: Andres Martinez Gotor <andres@bitnami.com>
Merging this. Thanks for the review! |
Requires #605
This PR adds a
/search
endpoint to the chartsvc to mimic the same behavior that monocular does in the client side:https://github.com/helm/monocular/blob/master/frontend/src/app/shared/services/charts.service.ts#L75
I have tested the changes manually but since the function is mostly a mongodb query there is nothing valuable that we can add as unit tests.