Skip to content
This repository has been archived by the owner on May 12, 2021. It is now read-only.

Show most popular charms instead of featured from apiv4 #677

Merged
merged 6 commits into from Dec 16, 2014

Conversation

hatched
Copy link
Contributor

@hatched hatched commented Dec 12, 2014

This branch removes the featured/interesting/new charms from the charm list in the GUI which were being pulled from apiv3 and replaces it with the search results from the 'most popular' charms from apiv4.

@hatched
Copy link
Contributor Author

hatched commented Dec 12, 2014

To QA

You should no longer see Featured, Interesting, and New in the charmbrowser on initial load. Instead you should see the most popular charms.

@kadams54
Copy link
Contributor

👍 on the code. Found a dupe showing between recommended and others during QA.

Recommended: http://cl.ly/image/2c2P3b1G221Y
Others: http://cl.ly/image/0w0d351V1E0C

Note that juju-gui/precise is in both lists. Once that's fixed, QA is OK.

@jujugui
Copy link
Contributor

jujugui commented Dec 12, 2014

Test PASSed.
Refer to this link for build results (access rights to CI server needed):
http://ci.jujugui.org:8080//job/juju-gui/2310/
Test PASSed.

@hatched
Copy link
Contributor Author

hatched commented Dec 13, 2014

I'm going to make some modifications to how this branch works before I land it:

@urosj
Copy link

urosj commented Dec 13, 2014

I'd like to chat about this before we implement new planned changes to te
API.

On Saturday, December 13, 2014, Jeff Pihach notifications@github.com
wrote:

I'm going to make some modifications to how this branch works before I
land it:

  • Filter the juju-gui charms out (They are in the GUI why do they need
    to deploy it again)
  • Only list popular promulgated charms (Ordered by the users default
    environment)
  • Shuffle the list (So that it stays fresh)
  • Switch to a single list template with the label "Popular"


Reply to this email directly or view it on GitHub
#677 (comment).

@mitechie
Copy link
Contributor

This involves no no changes to the API
On Dec 13, 2014 4:10 AM, "Uros Jovanovic" notifications@github.com wrote:

I'd like to chat about this before we implement new planned changes to te
API.

On Saturday, December 13, 2014, Jeff Pihach notifications@github.com
wrote:

I'm going to make some modifications to how this branch works before I
land it:

  • Filter the juju-gui charms out (They are in the GUI why do they need
    to deploy it again)
  • Only list popular promulgated charms (Ordered by the users default
    environment)
  • Shuffle the list (So that it stays fresh)
  • Switch to a single list template with the label "Popular"


Reply to this email directly or view it on GitHub
#677 (comment).


Reply to this email directly or view it on GitHub
#677 (comment).

@mitechie
Copy link
Contributor

To clarify this is just how the juju GUI will will use the search API
specifically in 1 view
On Dec 13, 2014 8:18 AM, "Rick Harding" rick.harding@canonical.com wrote:

This involves no no changes to the API
On Dec 13, 2014 4:10 AM, "Uros Jovanovic" notifications@github.com
wrote:

I'd like to chat about this before we implement new planned changes to te
API.

On Saturday, December 13, 2014, Jeff Pihach notifications@github.com
wrote:

I'm going to make some modifications to how this branch works before I
land it:

  • Filter the juju-gui charms out (They are in the GUI why do they need
    to deploy it again)
  • Only list popular promulgated charms (Ordered by the users default
    environment)
  • Shuffle the list (So that it stays fresh)
  • Switch to a single list template with the label "Popular"


Reply to this email directly or view it on GitHub
#677 (comment).


Reply to this email directly or view it on GitHub
#677 (comment).

@hatched
Copy link
Contributor Author

hatched commented Dec 14, 2014

Right, @urosj these are only changes to how the GUI will use the data from the query (link) above.

// get the promulgated version in the results list. This conditional
// can be removed once that bug is fixed in the charmstore. Est around
// the end of Jan 2015
if (entity.Meta['extra-info']['bzr-owner'] === 'charmers' &&
Copy link
Contributor

Choose a reason for hiding this comment

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

it needs to check if the id has a ~ or not

Copy link
Contributor Author

Choose a reason for hiding this comment

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

The bzr-owner field doesn't have ~ in it.

Copy link
Contributor

Choose a reason for hiding this comment

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

I know I mean the id

Copy link
Contributor

Choose a reason for hiding this comment

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

I am talking of the I'd field

Copy link
Contributor

Choose a reason for hiding this comment

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

this isn't valid?

Jeff Pihach added 2 commits December 15, 2014 11:41
@jujugui
Copy link
Contributor

jujugui commented Dec 15, 2014

Test PASSed.
Refer to this link for build results (access rights to CI server needed):
http://ci.jujugui.org:8080//job/juju-gui/2311/
Test PASSed.

@jujugui
Copy link
Contributor

jujugui commented Dec 15, 2014

Test PASSed.
Refer to this link for build results (access rights to CI server needed):
http://ci.jujugui.org:8080//job/juju-gui/2312/
Test PASSed.

@kadams54
Copy link
Contributor

QA is good now - the changes make it much better. Thanks!

@hatched
Copy link
Contributor Author

hatched commented Dec 16, 2014

:shipit: Thanks for the reviews and qa's!

@jujugui
Copy link
Contributor

jujugui commented Dec 16, 2014

Status: merge request accepted. Url: http://ci.jujugui.org:8080/job/juju-gui-merge

jujugui added a commit that referenced this pull request Dec 16, 2014
Show most popular charms instead of featured from apiv4

This branch removes the featured/interesting/new charms from the charm list in the GUI which were being pulled from apiv3 and replaces it with the search results from the 'most popular' charms from apiv4.
@jujugui jujugui merged commit aea2d9c into juju:develop Dec 16, 2014
@hatched hatched deleted the interesting-charmlist branch January 13, 2015 16:11
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Development

Successfully merging this pull request may close these issues.

None yet

5 participants