-
Notifications
You must be signed in to change notification settings - Fork 8
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
795 delete repository asynchonously #1028
Conversation
@@ -6,7 +6,7 @@ class HomeController < ApplicationController | |||
def index | |||
@comments = Comment.latest.limit(10).all | |||
@versions = OntologyVersion.latest.where(state: 'done').limit(10).all | |||
@repositories = Repository.accessible_by(current_user).latest.limit(10).all | |||
@repositories = Repository.active.accessible_by(current_user).latest.limit(10).all |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Metrics/LineLength: Line is too long. [86/80]
e8ea1c6
to
a01d764
Compare
a01d764
to
1578ed2
Compare
|
||
def is_destroying? | ||
self.class.instance_variable_get(:@destroying).include?(self.id) | ||
class Repository::Error < StandardError; end |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Style/ClassAndModuleChildren: Use nested module/class definitions instead of compact style.
88e4fad
to
89a1929
Compare
@@ -37,6 +37,6 @@ def filters_map | |||
private | |||
|
|||
def repository | |||
Repository.find_by_path params[:repository_id] | |||
Repository.active.find_by_path params[:repository_id] |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
please add the missing parentheses.
237763d
to
f50cbbd
Compare
@@ -11,7 +11,12 @@ class Repository < ActiveRecord::Base | |||
include Repository::Symlink | |||
include Repository::Validations | |||
|
|||
<<<<<<< HEAD |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
unexpected token tLSHFT
f50cbbd
to
34fec7a
Compare
a208f1c
to
e923e3e
Compare
def unmark_as_destroying | ||
self.class.instance_variable_get(:@destroying).delete(self.id) if self.id | ||
def can_be_deleted? | ||
ontologies.map(&:can_be_deleted_with_whole_repository?).all? |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Why don't you use each
so you don't have to run over all ontologies?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Good point. I seem to have been in lazy Haskell mode.
I could have introduced a default scope, but this would result in broken migrations: Some migrations change existing repositories. They would fail at selecting repositories because there is no column 'destroying'. Also default scopes cannot be overridden.
When destroying a repository asynchronously, we don't need to mark it as destroying in a class instance variable any more. Instead, we mark it in the database.
This reverts commit d053813. Conflicts: app/controllers/home_controller.rb app/controllers/ontology_search_controller.rb app/helpers/application_helper.rb app/models/repository/destroying.rb
Repositories marked as deleted shall still be browseable by the owner. If there is a default scope, many controllers and other parts of ontohub fail finding such a repo.
c9ba3f7
to
15c14e6
Compare
…usly 795 delete repository asynchonously
Shall fix #795 and #342.
I created this branch on top of #1026, not on top of staging.