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

Track persistence independent of presence of an identifier #339

Merged
merged 1 commit into from Jan 10, 2018

Conversation

Projects
None yet
2 participants
@jcoyne
Contributor

jcoyne commented Dec 20, 2017

Fixes a loop when adding ActiveModel::Conversion because conversion
defines to_param in terms of persisted?, which Valkyrie previously
defined in terms of to_param

Fixes #338

Track persistence independent of presence of an identifier
Fixes a loop when adding `ActiveModel::Conversion` because conversion
defines `to_param` in terms of `persisted?`, which Valkyrie previously
defined in terms of `to_param`

Fixes #338
def convert!
@resource ||= attributes[:internal_resource].constantize.new(attributes)
@resource ||= resource

This comment has been minimized.

@jcoyne

jcoyne Dec 20, 2017

Contributor

I did this refactor to make the postgres/orm_converter more like the solr/orm_converter which has these additional methods

@jcoyne

jcoyne Dec 20, 2017

Contributor

I did this refactor to make the postgres/orm_converter more like the solr/orm_converter which has these additional methods

@@ -20,11 +20,12 @@ def self.inherited(subclass)
subclass.attribute :internal_resource, Valkyrie::Types::Any.default(subclass.to_s)
subclass.attribute :created_at, Valkyrie::Types::DateTime.optional
subclass.attribute :updated_at, Valkyrie::Types::DateTime.optional
subclass.attribute :new_record, Types::Bool.default(true)

This comment has been minimized.

@jcoyne

jcoyne Dec 20, 2017

Contributor

I'd rather not have made this a schema attribute, but due to the use of constructor_type :schema I don't see any way around it. Suggestions?

@jcoyne

jcoyne Dec 20, 2017

Contributor

I'd rather not have made this a schema attribute, but due to the use of constructor_type :schema I don't see any way around it. Suggestions?

@tpendragon

This comment has been minimized.

Show comment
Hide comment
@tpendragon

tpendragon Jan 8, 2018

Collaborator

I need to test this on a running app to see what happens.

Collaborator

tpendragon commented Jan 8, 2018

I need to test this on a running app to see what happens.

@tpendragon tpendragon merged commit 8c197d8 into master Jan 10, 2018

2 checks passed

ci/circleci Your tests passed on CircleCI!
Details
coverage/coveralls Coverage increased (+0.002%) to 99.737%
Details

@tpendragon tpendragon deleted the persisted branch Jan 10, 2018

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment