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

[MNOE-1287] Delisted apps #785

Open
wants to merge 3 commits into
base: 4.0
Choose a base branch
from

Conversation

adamaziz15
Copy link
Contributor

Update tenant config schema to include only active apps

@adamaziz15 adamaziz15 added the bug label Sep 16, 2018
@adamaziz15 adamaziz15 added this to the Opal SoW2 Wk 37 milestone Sep 16, 2018
@adamaziz15 adamaziz15 requested a review from ouranos September 16, 2018 21:58
@adamaziz15 adamaziz15 force-pushed the bug/1287-delisted-apps branch from cc5dcdf to b0a71a3 Compare September 16, 2018 22:54
ouranos
ouranos previously approved these changes Sep 17, 2018
Update tenant config schema to include only active apps
@adamaziz15
Copy link
Contributor Author

@ouranos Rebased on 4.0

ouranos
ouranos previously approved these changes Jan 3, 2019
@@ -156,9 +156,9 @@ def self.update_locales_list!
# Populate the app list after fetching it from MnoHub
# TODO: replace with a Proc.call
def self.update_application_list!
available_app_nids = App.all.map(&:nid)
available_app_nids = App.where(active: true).map(&:nid)
Copy link
Contributor

Choose a reason for hiding this comment

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

Should we add a default scope to App?
I don't think mnoe should ever see inactive apps?

Copy link
Contributor

Choose a reason for hiding this comment

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

Or it could be done on the hub side

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Makes sense, maybe best to add it as a filter at the resource level in hub?

Copy link
Contributor

Choose a reason for hiding this comment

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

Yeah, should be by default I reckon?

The inactive apps shouldn't be exposed on the API. We only need them in the DevPlatformSync job to re-enable them if they get re-activated from the DevPlatform

@adamaziz15 adamaziz15 force-pushed the bug/1287-delisted-apps branch from c6b40f8 to d266abc Compare January 3, 2019 16:32
Add temp workaround for travis bundler issue. To be removed once issue
has been resolved.
@adamaziz15
Copy link
Contributor Author

adamaziz15 commented Jan 3, 2019

@ouranos Updated specs, looks like they were failing due to updates in nokogiri or sanitize gems. Build was then failing due to bundler failures in travis caused by outdated rubygems. I've added a temp fix for this until it is fixed in travis (added gem update --system to before_install: in travis config). See travis-ci/travis-ci#8969

@adamaziz15 adamaziz15 requested a review from ouranos January 3, 2019 17:39
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants