Skip to content

Conversation

@JoseWhite
Copy link
Collaborator

@JoseWhite JoseWhite commented Apr 10, 2025

Items Addressed

  • Initial migration, controllers and factories added

Out of Scope

  • Any item that should not be included in QA

Author Checklist

  • Review the diff and ensure all changes are relevant, intentional, and address all in-scope requirements
  • Assign a dev reviewer when you're ready for code review
  • Update environment variables as needed on Heroku and opensesame
  • Turn on workers on Heroku as needed
  • Assign a product reviewer when you're ready for QA (if applicable)

@JoseWhite JoseWhite requested a review from Copilot April 10, 2025 18:46
Copy link

Copilot AI left a comment

Choose a reason for hiding this comment

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

Copilot reviewed 25 out of 25 changed files in this pull request and generated 3 comments.

FactoryBot.define do
factory :room do
invite_code { SecureRandom.hex(10) }
venue { venue }
Copy link

Copilot AI Apr 10, 2025

Choose a reason for hiding this comment

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

The assignment 'venue { venue }' may lead to an unintended recursive reference. Consider using 'association :venue' or providing an explicit value to properly set up the association.

Suggested change
venue { venue }
association :venue

Copilot uses AI. Check for mistakes.
factory :room do
invite_code { SecureRandom.hex(10) }
venue { venue }
host_id { attendee }
Copy link

Copilot AI Apr 10, 2025

Choose a reason for hiding this comment

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

Assigning 'host_id' with 'attendee' is ambiguous; if host_id is intended to reference an Attendee's id, use an association (e.g., 'association :attendee') or assign the proper value explicitly.

Suggested change
host_id { attendee }
association :attendee, factory: :attendee, strategy: :build
host_id { attendee.id }

Copilot uses AI. Check for mistakes.
class CreateVotes < ActiveRecord::Migration[7.1]
def change
create_table :votes do |t|
t.references :rooms_venues, null: false, foreign_key: true
Copy link

Copilot AI Apr 10, 2025

Choose a reason for hiding this comment

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

The reference name 'rooms_venues' is plural; Rails conventions suggest using a singular name (e.g., 'room_venue') to properly define the belongs_to association in the Vote model.

Suggested change
t.references :rooms_venues, null: false, foreign_key: true
t.references :room_venue, null: false, foreign_key: true

Copilot uses AI. Check for mistakes.
@JoseWhite JoseWhite merged commit 4848808 into main Apr 16, 2025
2 of 8 checks passed
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.

2 participants