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

Use installation monitoring view instead of monitoring grid #4833

Merged
merged 23 commits into from Oct 30, 2017

Conversation

Projects
None yet
3 participants
@guerler
Contributor

guerler commented Oct 19, 2017

Branched from #4818. Currently the toolshed uses two separate types of installation monitoring views although they have the same title and content. The title is 'Monitor installing tool shed repositories'. One is a table and the other a grid mako. One is shown after successfully triggering an installation, the other when clicking on Monitor installing repositories in the admin panel. This PR removes the customized grid view and redirects calls to the table. Additionally the manage_repositories endpoint is renamed to install_repositories for clarification and now strictly returns json instead of html. This revises the last of three toolshed mako grids used in Galaxy's admin panel.

@guerler guerler added this to the 18.01 milestone Oct 19, 2017

@guerler guerler changed the title from Use regular installation monitoring view instead of monitoring grid to Use installation monitoring view instead of monitoring grid Oct 19, 2017

guerler added some commits Oct 19, 2017

@guerler guerler removed the status/WIP label Oct 25, 2017

@dannon

This comment has been minimized.

Member

dannon commented Oct 30, 2017

A nice step further would be to consolidate these two separate into a single filterable interface that just shows 'installing' repositories after the install step.

@dannon dannon merged commit 620ef86 into galaxyproject:dev Oct 30, 2017

6 checks passed

api test Build finished. 304 tests run, 4 skipped, 0 failed.
Details
continuous-integration/travis-ci/pr The Travis CI build passed
Details
framework test Build finished. 162 tests run, 0 skipped, 0 failed.
Details
integration test Build finished. 57 tests run, 0 skipped, 0 failed.
Details
lgtm analysis: JavaScript No alert changes
Details
toolshed test Build finished. 577 tests run, 0 skipped, 0 failed.
Details
@guerler

This comment has been minimized.

Contributor

guerler commented Oct 30, 2017

The underlying controller endpoint is the same. We could remove the link from the admin panel but then users would not be able to navigate back to the installation monitoring once they navigated away from it.

@dannon

This comment has been minimized.

Member

dannon commented Oct 30, 2017

@guerler Yeah, the endpoint being the same is why I think it'd be nice to group it all together more tightly instead of having two views.

@martenson

This comment has been minimized.

Member

martenson commented Oct 30, 2017

This might have introduced a regression where the 'messages' are not formatted. See screenshot.

screenshot 2017-10-30 15 14 22

@guerler

This comment has been minimized.

Contributor

guerler commented Oct 30, 2017

Thanks for the details. I agree, that makes sense.

@guerler

This comment has been minimized.

Contributor

guerler commented Oct 30, 2017

I'll look into it. Thx @martenson.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment