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

617 handle file deletion #1075

Merged
merged 19 commits into from
May 19, 2015
Merged

617 handle file deletion #1075

merged 19 commits into from
May 19, 2015

Conversation

eugenk
Copy link
Member

@eugenk eugenk commented Oct 8, 2014

This shall fix #617 and #958.

Files in the repository can be deleted now via the web interface.
Then, their ontologies will be marked as 'not having a file' (has_file).
This flag is stored in the database, but only for the head-version of an ontology (as a cache).
One can still ask for a specific commit, if an ontology has a file (has_file(commit_oid)), which then accesses the hard drive.

Deletion via ssh is detected and the flag is set in that case.

In the view (ontology#show), the nonexistence of a corresponding file is indicated by text and the absence of a link to the file.

@@ -209,6 +213,16 @@ def suspended_save_ontologies(options={})
schedule_batch_parsing(versions)
end

def mark_ontology_as_having_file(path, has_file)
ontos = ontologies.with_path(path)
if ontos.any?{ |onto| onto.has_file != has_file }

Choose a reason for hiding this comment

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

Style/GuardClause: Use a guard clause instead of wrapping the code inside a conditional expression.
Style/SpaceBeforeBlockBraces: Space missing to the left of {.

@@ -55,4 +55,10 @@ def status(resource)
html
end

def last_file_path(resource)
repository_ref_path(repository_id: resource.repository.to_param,
ref: resource.current_version.commit_oid,

Choose a reason for hiding this comment

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

Style/AlignHash: Align the elements of a hash literal if they span more than one line.

@eugenk eugenk force-pushed the 617-handle_file_deletion branch 2 times, most recently from 22a29d8 to 8bbf5c7 Compare December 4, 2014 11:01
@eugenk eugenk force-pushed the 617-handle_file_deletion branch 2 times, most recently from e9bb215 to 5931d1e Compare February 3, 2015 08:58

def has_file(commit_oid = nil)
if repository.is_head?(commit_oid)
read_attribute(:has_file)

Choose a reason for hiding this comment

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

Rails/ReadWriteAttribute: Prefer self[:attr] over read_attribute(:attr).

@eugenk eugenk force-pushed the 617-handle_file_deletion branch 2 times, most recently from d3dec09 to f256897 Compare February 3, 2015 09:03
redirect_to fancy_repository_path(repository, path: resource.target_path)
else
render :show
end
end

def destroy

Choose a reason for hiding this comment

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

Assignment Branch Condition size for destroy is too high. [16/15]

Copy link
Member Author

Choose a reason for hiding this comment

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

I'm not sure if this is a hound/rubocop bug or I just don't understand this comment. There is no branching in this method.

@eugenk
Copy link
Member Author

eugenk commented Feb 18, 2015

I'm ignoring the hound comments on routes.rb/spacing because then the file would look more inconsistent. The double quotes comments are just wrong. Seems to be a bug in hound/rubocop.

dir = nil
Array(0..dirs.length - 1).reverse.each do |i|
if dir.nil?
path = dirs[0..i].join('/')

Choose a reason for hiding this comment

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

Prefer double-quoted strings unless you need single quotes to avoid extra backslashes for escaping.

@eugenk eugenk force-pushed the 617-handle_file_deletion branch 2 times, most recently from cca0a1d to f792d42 Compare March 4, 2015 21:06
@0robustus1
Copy link
Contributor

please rebase staging.

end

it 'create a new ontology version' do
expect(repository.ontologies.
Copy link
Contributor

Choose a reason for hiding this comment

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

Indentation and line-breaks seem strange.

Copy link
Member Author

Choose a reason for hiding this comment

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

Fixed with 69b618d.

@eugenk
Copy link
Member Author

eugenk commented May 7, 2015

Tests are red because #1343's commits are not in here.

@0robustus1
Copy link
Contributor

So tests should be green if i merge #1343 locally and then run the tests?

@eugenk
Copy link
Member Author

eugenk commented May 7, 2015

Yes. And if you have the latest hets version (v0.99, 1430921534), of course.

@0robustus1
Copy link
Contributor

👍

eugenk added a commit that referenced this pull request May 19, 2015
@eugenk eugenk merged commit 207be30 into staging May 19, 2015
@eugenk eugenk deleted the 617-handle_file_deletion branch May 19, 2015 12:39
@eugenk eugenk mentioned this pull request Aug 24, 2015
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

deleting files/repositories
3 participants