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

New User Preferences User Interface #3118

Merged
merged 256 commits into from Nov 30, 2016

Conversation

@anuprulez
Copy link
Member

commented Nov 2, 2016

New User Preferences User Interface

This Pull Request incorporates new UI for User Preferences built using JavaScript and Python and replaces mako files. A Mako file has client and server side codes making it cumbersome to understand and maintain. So, this PR introduces a cleaner way to retire the mako files (for User Preferences) by using just one JavaScript file which contains generic code for pulling/ sending data from/ to the server for all the features (like Manage User Information, Change Password etc.). Moreover, it uses inbuilt Form view (Backbone view) to build those features. The Python script contains all the API calls, builds input attributes required for these Form views and does all the input validations before saving them.

The first look:

How you can activate it:

In the galaxy.ini config file (/config/galaxy.ini), there is a variable:
enable_new_user_preferences which has to be set to True to get an option 'New User Preferences' under User menu. By default, it is set to False.

Credits:

A big thanks to Aysam (@guerler) and Björn (@bgruening) who led and supervised at every step to bring it to this stage.

All suggestions are welcome. We wish the suggestions would bring more out of it and lead us to a better way to replace mako files.

Note: It fixes a bug of empty public name being saved.

anuprulez and others added some commits Aug 1, 2016

Merge pull request #16 from guerler/remove_mako_user_pref_form_view
Use form view to build change password form
Merge pull request #18 from guerler/fix_communication_settings
Remove unused field value update, and api request parameters

@guerler guerler force-pushed the bgruening:remove_mako_user_pref branch from ff80d75 to f51cf9f Nov 28, 2016

@guerler guerler force-pushed the bgruening:remove_mako_user_pref branch from f51cf9f to a4b70aa Nov 28, 2016

@guerler guerler added status/review and removed status/WIP labels Nov 28, 2016

@martenson

This comment has been minimized.

Copy link
Member

commented Nov 28, 2016

@galaxybot test this

@martenson

This comment has been minimized.

Copy link
Member

commented Nov 29, 2016

  • http://127.0.0.1:8080/user shows the old pref UI (the new one is under /users - I would prefer to drop the 's')
  • on multiple places there is wrong tooltip for the top right button (also in this case -API key- the icon is confusing)
    screenshot 2016-11-29 14 59 06
  • It should be made clear that user has to have ALL the roles specified here to access the history. This caused confusion in the past.
    screenshot 2016-11-29 14 59 35

Overall this is very nice, thank you for the improvements and hoping to merge soon.

@martenson

This comment has been minimized.

Copy link
Member

commented Nov 29, 2016

once we merge this we can work towards something like this:
screenshot 2016-11-29 15 21 42

@martenson

This comment has been minimized.

Copy link
Member

commented Nov 30, 2016

After seeing 57fb159 I am now confused (and I have never used the settings so bare with me please). Is the default permissions set up here (user prefs) affecting:

  • New Histories themselves
  • New datasets in any history
  • New datasets in new history
  • Something else
  • Combination of the above

?

@martenson

This comment has been minimized.

Copy link
Member

commented Nov 30, 2016

After some testing it seems the third option "New datasets in new history" is the one.

@martenson

This comment has been minimized.

Copy link
Member

commented Nov 30, 2016

I propose to add a explanation that summarizes the above to the use prefs page:

The following setting define the default 'Dataset Security' options for newly created histories. In other words, changes made on this page will affect permissions on new datasets - however this is only true in histories created after these settings were set.

@martenson

This comment has been minimized.

Copy link
Member

commented Nov 30, 2016

Thanks all for the contributions! 🎉

@martenson martenson merged commit b165757 into galaxyproject:dev Nov 30, 2016

4 checks passed

api test Build finished. 232 tests run, 0 skipped, 0 failed.
Details
continuous-integration/travis-ci/pr The Travis CI build passed
Details
framework test Build finished. 126 tests run, 0 skipped, 0 failed.
Details
toolshed test Build finished. 580 tests run, 0 skipped, 0 failed.
Details
@jmchilton

This comment has been minimized.

Copy link
Member

commented Nov 30, 2016

Awesome sauce all - great to see such a collaboration!

@guerler

This comment has been minimized.

Copy link
Contributor

commented Nov 30, 2016

I totally agree. Thanks a lot everyone, particularly for the very constructive review and useful comments.

@bgruening bgruening deleted the bgruening:remove_mako_user_pref branch Dec 3, 2016

@bgruening

This comment has been minimized.

Copy link
Member

commented Dec 3, 2016

@guerler thanks a lot! I have no words, thanks for taking up this task, getting it merged and showing us the right direction.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
You can’t perform that action at this time.