Refactors the PlumSchema into Schema Modules #79
Conversation
app/models/schema/plum/hyrax.rb
Outdated
| module Plum | ||
| ## | ||
| # Defines the attributes migrated from Hyrax core | ||
| module Hyrax |
There was a problem hiding this comment.
I am wary of naming any of our modules or classes "Hyrax" — mostly for symbolic reasons and to avoid confusion if/when Hyrax support for Valkyrie is undertaken.
I also think it would be nice to split up the terms by source vocabulary: many of these are from DC/DCTerms or MARC Relators.
app/models/schema/plum/imported.rb
Outdated
| def self.attributes | ||
| [ | ||
| # Used for a license, | ||
| :license, |
There was a problem hiding this comment.
As with the comment above, breaking these out by source would be nice.
app/models/schema/plum/local.rb
Outdated
| module Plum | ||
| ## | ||
| # Defines the attributes local to Plum | ||
| module Local |
There was a problem hiding this comment.
Same as above: breaking these out by source would be nice, since some of these properties are our own local properties, but some (alternative, identifier, etc.) are DC.
There was a problem hiding this comment.
Apologies, I misunderstood this from within #7 - I can address this as well.
|
|
||
| included do | ||
| Common.attributes.each do |field| | ||
| attribute field |
There was a problem hiding this comment.
Is there a way to avoid this boilerplate code in each schema?
There was a problem hiding this comment.
That was my primary struggle with this PR, I'll definitely see about this immediately.
There was a problem hiding this comment.
It's fine if it's not possible — or we could make a separate ticket for looking into that later so it doesn't block this work.
There was a problem hiding this comment.
Apologies, it is likely to continue to block this work, as I would likely need to abandon any (simple) approach using Modules. Within the scope of included, I do not believe that I can easily reference the Module being mixed into the Class. From what I could determine, this is not a limitation of ActiveSupport::Concern:
https://gist.github.com/jrgriffiniii/8cf9ab64d4ecaff44a2f59284d23ba85
There was a problem hiding this comment.
You could always make them not modules - add a method like apply_schema to Valhalla::Resource (or via a module I guess) and each schema be a class with an interface that apply_schema can utilize.
There was a problem hiding this comment.
Agreed, something similar was what I was considering for #80 , but I was wary of further blocking this issue.
| @@ -1,5 +1,25 @@ | |||
| # frozen_string_literal: true | |||
| class Valkyrie::ResourceDecorator < ApplicationDecorator | |||
| self.suppressed_attributes = [ | |||
There was a problem hiding this comment.
I'm not sure I understand what's going on here, since at first blush these look like attributes we'd want to display. Is this related to moving the imported attributes to a nested resource, so these get suppressed on the parent resource?
There was a problem hiding this comment.
No, I will document this in the code base for clarity. This an attempt to structure the following View logic from Plum:
https://github.com/pulibrary/plum/blob/master/app/views/hyrax/base/_attributes.html.erb#L8
https://github.com/pulibrary/plum/blob/master/app/schemas/plum_schema.rb#L302
jrgriffiniii
force-pushed
the
issues-20-jrgriffiniii-add-fields-view-sr-mvw
branch
4 times, most recently
from
96fc69a
to
b2a39ed
Compare
jrgriffiniii
changed the title
jrgriffiniii
changed the title
jrgriffiniii
force-pushed
the
issues-20-jrgriffiniii-add-fields-view-sr-mvw
branch
from
b2a39ed
to
6f35eb6
Compare
jrgriffiniii
changed the title
…e Schema:: Modules; Adds explicitly defined "suppressed" fields for decorated Valkyrie Resources Migrating the refactored Schema Modules into the ImportedMetadata Class Addressing styling rules and updating the RuboCops for the Schema Modules Restructuring the attributes based upon their sources (where possible) Tracking the new schema Modules Excluding the Schema Modules from the coverage analysis Extending the test suites for the schema Modules; Extending the test for the ScannedResource View displayed attributes Resolving styling issues Removing extraneous legacy attributes from being suppressed Removing the unneeded Schema::Schema Module
jrgriffiniii
force-pushed
the
issues-20-jrgriffiniii-add-fields-view-sr-mvw
branch
from
6f35eb6
to
2c34528
Compare
Resolves #20 and #7 by addressing the following:
Schema::*Modules