More detailed file status #66
Conversation
little9
force-pushed
the
individual-file-status
branch
from
a86e018
to
0de8a55
Compare
| def status_icon(status) | ||
| # rubocop:disable Rails/OutputSafety | ||
| return "<span class='glyphicon glyphicon-ok-sign text-success'></span>".html_safe if status == 'attached' | ||
| "<span class='glyphicon glyphicon-question-sign'></span>".html_safe |
There was a problem hiding this comment.
Is this idiom preferred (return if)? Otherwise if there are just two equal options (this or that) I find it a little easier to read if/else structures.
There was a problem hiding this comment.
When we use icons semantically in this way, we should generally include an aria-label see https://fontawesome.com/how-to-use/on-the-web/other-topics/accessibility
There was a problem hiding this comment.
@mark-dce There's a rubocop/bixby rule we have for this: https://www.rubydoc.info/gems/rubocop/RuboCop/Cop/Style/GuardClause
https://github.com/samvera-labs/bixby/blob/master/bixby_default.yml#L129
It will fail if you use an if / else block when you could have used a guard clause.
config/initializers/prepends.rb
Outdated
| @@ -0,0 +1 @@ | |||
| AttachFilesToWorkJob.prepend(AttachFilesToWorkJobPrepends) | |||
There was a problem hiding this comment.
Could we use this same strategy to add additional logging to Hyrax background jobs?
There was a problem hiding this comment.
Yeah, and doing that way avoids needing to override it which could be problematic if someone is already overriding a job.
spec/support/hyrax/basic_metadata.rb
Outdated
| @@ -25,6 +25,9 @@ def self.included(work) | |||
| work.property :related_url, predicate: ::RDF::RDFS.seeAlso | |||
| work.property :bibliographic_citation, predicate: ::RDF::Vocab::DC.bibliographicCitation | |||
| work.property :source, predicate: ::RDF::Vocab::DC.source | |||
|
|
|||
| # From Zizia | |||
| work.property :deduplication_key, predicate: 'http://metadata.example.com/vocab/predicates#deduplicationKey' | |||
There was a problem hiding this comment.
I'm wondering if we just want to implement a primary_id and find a better predicate. It feels like that's what most adopters are going to want anyway.
How about :RDF::VOCAB::BF2::identifiedBy
There was a problem hiding this comment.
I think adding a more specific predicate may ultimately be confusing because then institutions may be tempted to use it for some other purpose. Right now the the deduplication_key is something that's only used for grouping the Work and FileSets during the import.
There was a problem hiding this comment.
I changed the predicate to ::RDF::VOCAB::BF2.identifiedBy, but I really think we should keep using the name deduplication_key since Emory is already using that.
little9
force-pushed
the
individual-file-status
branch
2 times, most recently
from
18eed87
to
5520940
Compare
little9
force-pushed
the
individual-file-status
branch
2 times, most recently
from
d066b75
to
661545d
Compare
|
|
little9
force-pushed
the
individual-file-status
branch
5 times, most recently
from
fc797b1
to
86000db
Compare
This adds a status field to `PreIngestFile`s and `PreIngestWorks` that defaults to 'preingest'. The `ContentDepositEventJob` and `FileSetAttachedEventJob` classes are preprended with logic that updates their status to 'attached'. The logic requires that `deduplication_key` is present in the CSV so that field is added to the default mapper and validator. There is a helper which outputs a success icon (a green checkmark) if a `PreIngestFile` or `PreIngestWork` has an 'attached' status.
little9
force-pushed
the
individual-file-status
branch
from
86000db
to
3004568
Compare
This adds a status field to
PreIngestFiles thatdefaults to 'preingest'.
The
AttachFilesToWorkJobclass is preprended withlogic that updates the status to 'attached'.
The logic requires that
deduplication_keyis present in theCSV so that field is added to the default mapper and validator.
There is a helper which outputs a success icon (a green checkmark) if
the
PreIngestFilehas an 'attached' status. This is used for thePreIngestWorkif all the statuses for thePreIngestFileare 'attached'.