ondemand

ondemand copied to clipboard

Currently a sub app token like: sys/rstudio/big_memory matches application: sys/rstudio

This behaviour does not seem correct. This use case was discovered with a unit test.

expected behaviour: token: sys/rstudio, app: sys/rstudio => match :white_check_mark: token: sys/rstudio, app: sys/rstudio/big_memory => match :white_check_mark: token: sys/rstudio/big_memory, app: sys/rstudio => match :x: token: sys/rstudio/big_memory, app: sys/rstudio/big_memory => match :white_check_mark: token: sys/rstudio/big_memory, app: sys/rstudio/small_memory => match :x:

┆Issue is synchronized with this Asana task by Unito

Created small with some tests and a possible fix: https://github.com/OSC/ondemand/pull/2236

There are a lot of errors when selecting pinned_apps, maybe this is expected behaviour.

maybe this is expected behaviour.

I wouldn't say expected, I'd probably phrase it as good fit for the time being. We do have #1131 which may be specific to the shell app or it may also be this. I.e., you want to specify a main app and get all the sub apps too.

But I do believe that that's the behavior in navbar links. That is, in this example you'd see links for sys/rstudio/big_memory and sys/rstudio/small_memory but not for the parent app sys/rstudio.

I think I found the issue. When filtering apps using the TokenMatcher, it only process sub apps if there is a match for the parent app.

Should apps and sub apps be combined into a flat list and then apply the filtering?

Router class

def self.featured_apps_from_token(token, all_apps, category, subcategory)
    matcher = TokenMatcher.new(token)

    all_apps.select do |app|
      matcher.matches_app?(app) # ONLY SELECTED APPS WILL BE PROCESSED FOR SUB APPS
    end.each_with_object([]) do |app, apps|
      app_category = category || app.category
      app_subcategory = subcategory || app.subcategory

      if app.has_sub_apps?
        apps.concat(featured_apps_from_sub_app(app, matcher, app_category, app_subcategory))
      else
        apps.append(FeaturedApp.new(app.router, category: app_category, subcategory: app_subcategory))
      end
    end
  end

I have updated the PR to process sub_apps regardless of parent matching. Tests are now green.

Fixed by https://github.com/OSC/ondemand/pull/2236