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

There should be a warning if you specify two properties that share the same predicate. #662

Closed
jcoyne opened this issue Dec 22, 2014 · 5 comments
Assignees
Milestone

Comments

@jcoyne
Copy link
Member

jcoyne commented Dec 22, 2014

Would help avoid bugs like this:

     Failure/Error: let(:object) { TestModel.new(title: ['foo', 'bar'], creator: 'baz') }
     ActiveFedora::ConstraintError:
       Expected "creator" to have 0-1 statements, but there are 2
     # ./spec/presenters/hydra_editor_presenter_spec.rb:23:in `new'

Which was caused by title and creator accidentally having the same predicate

@jcoyne jcoyne added this to the 9.0.0 milestone Dec 22, 2014
@hectorcorrea hectorcorrea self-assigned this Jan 5, 2015
@jcoyne
Copy link
Member Author

jcoyne commented Jan 5, 2015

A predicate can be shared if each term has different class_name properties. So the warning should only be raised if the cardinality (multiple property) is different on both.

@hectorcorrea
Copy link
Member

@jcoyne A few questions.

  1. Why does the "class_name" matter?
  2. Is this the expected behavior for when two properties have the same predicate and

a) if the have the same "class_name" and different "multiple" - OK
b) if the have the same "class_name" and same "multiple" - Warn user
c) if the have the different "class_name" and different "multiple" - OK
d) if the have the different "class_name" and same "multiple" - OK

  1. If one (or both) of them have no "class_name" indicated apply the same logic?

@jcoyne
Copy link
Member Author

jcoyne commented Jan 9, 2015

Actually I think the class_name difference only applies to associations (belongs_to, has_many). I may have confused that in my head, so it should probably always warn on duplicate predicates regardless of how any other parameters are set.

@hectorcorrea
Copy link
Member

@jcoyne I have this note form our IRC chat the other day on which @terrellt brought up "class_name"

AT can handle property 

:title,   :class_name => Title1, :predicate => RDF::DC.title, 
:title_2, :class_name => Title2, :predicate => RDF::DC.title

if Title1 and Title2 have defined types which differ.

Is this why you included it perhaps? I've removed the "class_name" check in my PR but I wonder if this changes things.

@jcoyne
Copy link
Member Author

jcoyne commented Jan 10, 2015

Perhaps. It's not a use case I have, see what @terrellt has to say.

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

No branches or pull requests

2 participants