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

pinned apps to support wildcard globs #898

Merged
merged 8 commits into from
Feb 25, 2021
Merged

Conversation

johrstrom
Copy link
Contributor

@johrstrom johrstrom commented Feb 22, 2021

Router#pinned_apps now supports glob style wildcards like sys/* and
sys/bc_desktop/* (usr/ and dev/ included). This also adds public Router#apps
methods to be possibly useful in the future.

This also adds a OodApp#has_sub_apps? api to determine if an app
in fact has valid sub apps for showing the subapp in the pinned
apps menus.

I can comment in the tickets where behaviour is defined more specifically, but basically here are a few common use cases for this:

pinned_apps
   # get all the system and usr apps
   - "sys/*"
   - "usr/*"

   # show all the subapps for sys/bc_desktop
   - "sys/bc_desktop/*"

   # get all the apps from a specific usr share location
   - "usr/some_user/*"

@ericfranz
Copy link
Contributor

https://github.com/OSC/ondemand/pull/898/files#diff-60fef4a9a6a3bd5d06e005f9d166e5fb2522e5a740569fd3402cbe1c0e322213R4-R16 is duplicate of

def sys_apps
@sys_apps ||= SysRouter.apps
end
def dev_apps
@dev_apps ||= ::Configuration.app_development_enabled? ? DevRouter.apps : []
end
def usr_apps
@usr_apps ||= ::Configuration.app_sharing_enabled? ? UsrRouter.all_apps(owners: UsrRouter.owners) : []
end

To remove this duplication I would instead do this:

  1. add ApplicationController#all_apps which does sys_apps + usr_apps + dev_apps
  2. change Router.pinned_apps to accept 2 arguments: (tokens, all_apps)
  3. add ApplicationController#pinned_app_group which does OodAppGroup.groups_for(apps: Router.pinned_apps(Configuration.pinned_apps, all_apps))

Router#pinned_apps now supports glob style wildcards like sys/* and
sys/bc_desktop/* (usr/ and dev/ included). This also adds public Router#apps
methods to be possibly useful in the future.

This also adds a OodApp#has_sub_apps? api to determine if an app
in fact has valid sub apps for showing the subapp in the pinned
apps menus.
refactor pinned_apps to accept the tokens and apps as input and compute
pinned apps. This puts app logic/member variables in the controllers
instead of in Router
@johrstrom johrstrom force-pushed the wildcard-pinned-apps branch from 1e36d8f to b60b10e Compare February 23, 2021 16:33
@johrstrom
Copy link
Contributor Author

Done in 5059010. I had to rebase off of the current master because I removed the helper pinned_apps? because @pinned_apps is a member variable now of the controller now.

@johrstrom johrstrom merged commit 5dc7ea9 into master Feb 25, 2021
@johrstrom johrstrom deleted the wildcard-pinned-apps branch February 25, 2021 20:09
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants