-
Notifications
You must be signed in to change notification settings - Fork 0
IIIRR-29 Update for issue in tables and prevent answer duplication #23
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
Conversation
) | ||
return | ||
end | ||
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I removed this as it was repetitive.
embeds: [ answer_embed(answer, puzzle) ], | ||
ephemeral: true # Only the user sees this message | ||
) | ||
else |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
After looking further into this it seems as though answer
should always be nil if the validation fails or the db index unique fails, so the if/else should work here.
belongs_to :puzzle | ||
belongs_to :user | ||
|
||
validates :puzzle_id, uniqueness: { scope: [ :user_id, :server_id ] } |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Add an extra layer of validation
@@ -1,3 +1,4 @@ | |||
class Server < ApplicationRecord | |||
has_and_belongs_to_many :users, join_table: "users_servers" |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I am not sure how this was working before
has_and_belongs_to_many :servers | ||
has_many :servers | ||
has_and_belongs_to_many :servers, join_table: "users_servers" | ||
has_many :answers |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Remove repetitive servers
, I think this must have been a mistake and was supposed to be answers
.
@@ -0,0 +1,5 @@ | |||
class AddUniqueIndexToAnswers < ActiveRecord::Migration[8.0] |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Add migration with db uniqueness for each answer
@@ -1,19 +1,47 @@ | |||
|
|||
# db/seeds.rb | |||
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
While testing this in the rails console I was having several issues so I updated the seed file to more closely resemble the relationships.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
LGTM 🚀
This PR attempts to fix the issues we are seeing with the weekly leaderboard not having the correct scores, and multiple entries being created in the db for one answer.
I also noticed that there were some issues with the way that the active record models were configured when it came to their relationships, so I updated that as well.