Skip to content
Open
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
2 changes: 1 addition & 1 deletion app/models/arbitrary_assignment_creation.rb
Original file line number Diff line number Diff line change
Expand Up @@ -73,7 +73,7 @@ def superseding?
end

def individual_override?
superseding? && (assignment.individually_overridden || !bulk_assignment?)
superseding? && !bulk_assignment?
end

def bulk_assignment?
Expand Down
1 change: 1 addition & 0 deletions app/models/bulk_reassignment.rb
Original file line number Diff line number Diff line change
Expand Up @@ -65,6 +65,7 @@ def update_assignments # rubocop:disable Metrics/AbcSize
mixpanel_result = NULL,
bulk_assignment_id = #{connection.quote(bulk_assignment.id)},
visitor_supersession_id = NULL,
individually_overridden = false,
context = 'bulk_assignment',
force = #{connection.quote(bulk_assignment.force)}
WHERE id #{assignment_id_clause}
Expand Down
Original file line number Diff line number Diff line change
@@ -0,0 +1,14 @@
class ClearStaleIndividuallyOverriddenFlags < ActiveRecord::Migration[7.2]
def up
execute <<~SQL
UPDATE assignments
SET individually_overridden = false
WHERE individually_overridden = true
AND context IS DISTINCT FROM 'individually_overridden'

Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

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

I had an ephemeral test to validate that this migration behaved as expected, but it felt exotic to test a migration so I deleted it.

SQL
end

def down
raise ActiveRecord::IrreversibleMigration
end
end
2 changes: 1 addition & 1 deletion db/schema.rb
Original file line number Diff line number Diff line change
Expand Up @@ -10,7 +10,7 @@
#
# It's strongly recommended that you check this file into your version control system.

ActiveRecord::Schema[7.0].define(version: 2019_04_13_210327) do
ActiveRecord::Schema[7.2].define(version: 2026_05_04_164518) do
# These are extensions that must be enabled in order to support this database
enable_extension "plpgsql"
enable_extension "uuid-ossp"
Expand Down
6 changes: 3 additions & 3 deletions spec/models/arbitrary_assignment_creation_spec.rb
Original file line number Diff line number Diff line change
Expand Up @@ -233,15 +233,15 @@
end
end

it "remains during a bulk assignment" do

Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

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

This seemed like "testing behavior that isn't part of the requirements" when I paged through the archived repository. I couldn't find any documented reason that this would be desirable behavior. It basically committed to what I think is a bug here.

it "is cleared during a bulk assignment" do
existing_assignment = FactoryBot.create(:assignment, existing_assignment_params.merge(individually_overridden: true))
bulk_assignment = FactoryBot.create(:bulk_assignment, split: existing_assignment_params[:split])

ArbitraryAssignmentCreation.create!(params.merge(bulk_assignment_id: bulk_assignment.id))

expect(existing_assignment.reload.bulk_assignment).to eq bulk_assignment
expect(existing_assignment).to be_individually_overridden
expect(existing_assignment.context).to eq "individually_overridden"
expect(existing_assignment).not_to be_individually_overridden
expect(existing_assignment.context).to eq "bulk_assignment"
end

it "makes a previous bulk_assignment nil" do
Expand Down
4 changes: 2 additions & 2 deletions spec/models/bulk_assignment_creation_spec.rb
Original file line number Diff line number Diff line change
Expand Up @@ -289,15 +289,15 @@ def subject_with_reason(reason)
expect(previous_assignment.individually_overridden).to be false
end

it "maintains the individually_overridden dirty flag across bulk assignments" do
it "clears the individually_overridden flag across bulk assignments" do
assignment.update(individually_overridden: true, context: "individually_overridden")

subject.save

current_assignment = Assignment.find_by(visitor:)
previous_assignment = current_assignment.previous_assignments.first

expect(current_assignment.individually_overridden).to be true
expect(current_assignment.individually_overridden).to be false
expect(current_assignment.context).to eq "bulk_assignment"
expect(previous_assignment.individually_overridden).to be true
expect(previous_assignment.context).to eq "individually_overridden"
Expand Down
4 changes: 2 additions & 2 deletions spec/models/bulk_reassignment_spec.rb
Original file line number Diff line number Diff line change
Expand Up @@ -75,8 +75,8 @@
expect(assignment.mixpanel_result).to be_nil
end

it "preserves individually_overridden" do
expect(assignment.individually_overridden).to be true
it "clears individually_overridden" do
expect(assignment.individually_overridden).to be false
end

it "uses bulk_assignment's created_at as updated_at" do
Expand Down
Loading