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

store project_id directly on release for easier querying #912

Merged
merged 1 commit into from Apr 30, 2016

Conversation

grosser
Copy link
Contributor

@grosser grosser commented Apr 29, 2016

we are doing unnecessary queries in a few places and adding indirection ... also means we need joins ... just normalize it like most other things already do ...

@zendesk/paas

@@ -6,12 +6,11 @@ class Release < ActiveRecord::Base

belongs_to :user
belongs_to :build
belongs_to :project
Copy link
Collaborator

Choose a reason for hiding this comment

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

Can you add a validation that build.project_id == product_id?

Copy link
Collaborator

Choose a reason for hiding this comment

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

and maybe a before_validation filter that project_id ||= build.try(:project_id)

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 blows up when it's not set, so makes sure it is always passed in -> not
necessary to prefill it ? / no other model does that
no other model has project integrity validation -> not necessary here ?

On Fri, Apr 29, 2016 at 4:37 PM, Jon Moter notifications@github.com wrote:

In plugins/kubernetes/app/models/kubernetes/release.rb
#912 (comment):

@@ -6,12 +6,11 @@ class Release < ActiveRecord::Base

 belongs_to :user
 belongs_to :build
  • belongs_to :project

and maybe a before_validation filter that project_id ||=
build.try(:project_id)


You are receiving this because you authored the thread.
Reply to this email directly or view it on GitHub
https://github.com/zendesk/samson/pull/912/files/342ea153614715c3f9867aee6ef6c6f63bf8e11c#r61658036

@grosser
Copy link
Contributor Author

grosser commented Apr 29, 2016

added project integrity validation

@jonmoter
Copy link
Collaborator

Did you mean to make all those changes to schema.rb?

@grosser
Copy link
Contributor Author

grosser commented Apr 30, 2016

updated!

@jonmoter
Copy link
Collaborator

👍 on resolving merge conflicts

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.

None yet

2 participants