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

[WIP] Valkyrie #1777

Closed
wants to merge 352 commits into from
Closed

[WIP] Valkyrie #1777

wants to merge 352 commits into from

Conversation

jcoyne
Copy link
Member

@jcoyne jcoyne commented Oct 6, 2017

No description provided.

@jcoyne jcoyne force-pushed the valkyrie branch 2 times, most recently from e69fc02 to c85a2e1 Compare October 9, 2017 03:43
@@ -58,6 +56,16 @@ def apply_order(curation_concern, new_order)
curation_concern.list_source.order_will_change!
true
end

def find_resource(id)
query_service.find_by(id: Valkyrie::ID.new(id.to_s))
Copy link
Contributor

Choose a reason for hiding this comment

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

This looks like Valkyrie should be responsible for casting the given id to a Valkyrie:ID. This method does not know that the given query_service is Valkyrie. As such I believe the find_by method in Valkyrie should be responsible for casting to an id.

cjcolvar and others added 25 commits October 24, 2017 12:03
Adjust the DynamicChangeSet to namespace for Hyrax classes
Refactor count queries away from ActiveFedora::Relation
Works controller now uses change sets rather than actors
Remove the namespacing from the Change Set
Add a CollectionChangeSetPersister
Restore attributes for thumbnail and representative
Convert batch forms to ChangeSets
.dropdown-toggle:focus {
outline: 2px auto Highlight; // FireFox
outline: 5px auto -webkit-focus-ring-color; // Chrome, Safari
outline: -2px;

Choose a reason for hiding this comment

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

Property outline already defined on line 25


.dropdown-toggle:focus {
outline: 2px auto Highlight; // FireFox
outline: 5px auto -webkit-focus-ring-color; // Chrome, Safari

Choose a reason for hiding this comment

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

Property outline already defined on line 25

@@ -20,6 +20,10 @@ legend small {
}
}

#savewidget a {

Choose a reason for hiding this comment

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

Avoid using id selectors

.dropdown-toggle:focus {
outline: 2px auto Highlight; // FireFox
outline: 5px auto -webkit-focus-ring-color; // Chrome, Safari
outline: -2px;

Choose a reason for hiding this comment

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

Property outline already defined on line 25


.dropdown-toggle:focus {
outline: 2px auto Highlight; // FireFox
outline: 5px auto -webkit-focus-ring-color; // Chrome, Safari

Choose a reason for hiding this comment

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

Property outline already defined on line 25

@@ -20,6 +20,10 @@ legend small {
}
}

#savewidget a {

Choose a reason for hiding this comment

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

Avoid using id selectors

jcoyne and others added 18 commits November 9, 2017 20:10
Got rid of two views by moving the code to the CollectionsHelper too.
Restore the routing for the BatchUploadChangeSet
Reset permissions via the attributes on work
Use thumbnail_id rather than thumbnail
Cast strings to Valkyrie identifiers
Restore the stubbing of the actor in the WorksController spec
Remove autocreate_fields! it makes it impossible to set required fields
Actor returns the resource on successful update
…2166)

include ... order doesn't matter; added 'import_url'

foo
@jcoyne
Copy link
Member Author

jcoyne commented Nov 14, 2017

Closing this in favor of #2178

@jcoyne jcoyne closed this Nov 14, 2017
@jcoyne jcoyne deleted the valkyrie branch November 14, 2017 17:26
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.