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

Multiple attributes validates#244 #386

Merged
merged 2 commits into from
Nov 14, 2016

Conversation

jonnatas
Copy link
Contributor

isuue #244

@jonnatas
Copy link
Contributor Author

From what I understood, is that? @rafamanzo

@jonnatas jonnatas force-pushed the MultipleAttributesValidates#244 branch from 153da8b to 3a21d91 Compare October 17, 2016 18:43
@@ -6,6 +6,7 @@ The version numbers below try to follow the conventions at http://semver.org/.

## Unreleased

- Adding validades with presence uniqueness of ids
Copy link
Member

Choose a reason for hiding this comment

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

What do you think about: Adds validations (presence and uniqueness) of foreign keys?

@@ -1,6 +1,6 @@
class ProjectAttributes < ActiveRecord::Base
belongs_to :user
validates :project_id, presence: true
validates :project_id, presence: true, uniqueness: { scope: :user_id}
Copy link
Member

Choose a reason for hiding this comment

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

Why have you set this scope?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

It limits the scope in which uniqueness checking occurs, i.e., in this example, the project_id may repeat if the user_id is different. You think better cut this scope?

Copy link
Contributor

Choose a reason for hiding this comment

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

Thinking about this once more I don't believe we want to allow multiple project owners for now. Allowing that kind of behaviour may lead to expectations about features we do not yet implement (such as adding members to the project).

So maybe it's better to leave the scope out.

@@ -1,6 +1,6 @@
class RepositoryAttributes < ActiveRecord::Base
belongs_to :user
validates :repository_id, presence: true
validates :repository_id, presence: true, uniqueness: { scope: :user_id}
Copy link
Member

Choose a reason for hiding this comment

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

Why have you set this scope?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

the same here!

Copy link
Contributor

Choose a reason for hiding this comment

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

Thinking about this once more I don't believe we want to allow multiple project owners for now. Allowing that kind of behaviour may lead to expectations about features we do not yet implement (such as adding members to the project).

So maybe it's better to leave the scope out.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

ok 6fb9480

@jonnatas jonnatas force-pushed the MultipleAttributesValidates#244 branch from 3a21d91 to 609ac02 Compare October 28, 2016 23:53
@jonnatas jonnatas force-pushed the MultipleAttributesValidates#244 branch from 609ac02 to 2aa83f5 Compare November 8, 2016 14:50
@jonnatas jonnatas force-pushed the MultipleAttributesValidates#244 branch from 2aa83f5 to 6fb9480 Compare November 8, 2016 15:01
@jonnatas jonnatas force-pushed the MultipleAttributesValidates#244 branch from 6fb9480 to e2e4cd9 Compare November 8, 2016 17:05
@rafamanzo rafamanzo merged commit 193a92b into mezuro:master Nov 14, 2016
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.

3 participants