AO3-4128-more thorough progress fix #2489
+85
−22
2 participants
spec/models/potential_match_spec.rb
| + | ||
| +describe PotentialMatch do | ||
| + | ||
| + before do | ||
| + @potential_match = create(:potential_match) | ||
| + @collection = @potential_match.collection | ||
| + @first_signup = @collection.signups.first | ||
| + @last_signup = @collection.signups.last | ||
| + end | ||
| + | ||
| + it "should have a progress key" do | ||
| + expect(PotentialMatch.progress_key(@collection)).to include("#{@collection.id}") | ||
| + end | ||
| + | ||
| + it "should have a signup key" do | ||
| + expect(PotentialMatch.signup_key(@collection)).to include("#{@collection.id}") |
|
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
|
spec/models/potential_match_spec.rb
| @@ -0,0 +1,46 @@ | ||
| +require 'spec_helper' | ||
| + | ||
| +describe PotentialMatch do | ||
| + | ||
| + before do | ||
| + @potential_match = create(:potential_match) | ||
| + @collection = @potential_match.collection | ||
| + @first_signup = @collection.signups.first | ||
| + @last_signup = @collection.signups.last | ||
| + end | ||
| + | ||
| + it "should have a progress key" do | ||
| + expect(PotentialMatch.progress_key(@collection)).to include("#{@collection.id}") |
|
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
|
spec/models/potential_match_spec.rb
| @@ -0,0 +1,46 @@ | ||
| +require 'spec_helper' | ||
| + | ||
| +describe PotentialMatch do | ||
| + |
|
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
|
houndci-bot
commented on an outdated diff
factories/challenges.rb
| signup.offers.build(pseud_id: signup.pseud_id, collection_id: signup.collection_id) | ||
| signup.requests.build(pseud_id: signup.pseud_id, collection_id: signup.collection_id) | ||
| end | ||
| end | ||
| + | ||
| + factory :potential_match do |f| |
|
Unused block argument -
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
|
app/models/potential_match.rb
| @@ -20,8 +20,8 @@ def self.progress_key(collection) | ||
| CACHE_PROGRESS_KEY + "#{collection.id}" | ||
| end | ||
| - def self.byline_key(collection) | ||
| - CACHE_BYLINE_KEY + "#{collection.id}" | ||
| + def self.signup_key(collection) | ||
| + CACHE_SIGNUP_KEY + "#{collection.id}" |
|
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
|
houndci-bot
commented on an outdated diff
app/models/potential_match.rb
| @@ -4,7 +4,7 @@ class PotentialMatch < ActiveRecord::Base | ||
| ALL = -1 | ||
| CACHE_PROGRESS_KEY = "potential_match_status_for_" | ||
| - CACHE_BYLINE_KEY = "potential_match_bylines_for_" | ||
| + CACHE_SIGNUP_KEY = "potential_match_signups_for_" |
|
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
|
app/models/potential_match.rb
| @@ -20,8 +20,8 @@ def self.progress_key(collection) | ||
| CACHE_PROGRESS_KEY + "#{collection.id}" | ||
| end | ||
| - def self.byline_key(collection) | ||
| - CACHE_BYLINE_KEY + "#{collection.id}" | ||
| + def self.signup_key(collection) |
|
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
|
app/models/potential_match.rb
| @@ -104,9 +104,14 @@ def self.generate_in_background(collection_id) | ||
| required_types = settings.required_types.map {|t| t.classify} | ||
| # treat each signup as a request signup first | ||
| - collection.signups.find_each do |signup| | ||
| + # because find_each doesn't give us a consistent order, but we don't necessarily |
|
What ordering problem did you find with find_each? The Rails documentation says: "[Order] is automatically set to ascending on the primary key (“id ASC”) to make the batch ordering work." So I would expect that to do what you want? Actually, in retrospect I realized the issue was likely because the job-restarting code was causing the process to start over and over on top of itself. However, I think this remains a better way of handling the progress (doesn't break if user changes name/pseud midstream), so would still rather get this in.
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
|
2 checks passed
Details
continuous-integration/travis-ci/pr
The Travis CI build passed
hound
5 violations found.
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
https://otwarchive.atlassian.net/browse/AO3-4128
The progress report was NOT fixed. I've reworked the way it was being done, and also added tests. Now hopefully working for reals. Tests running clean.
Signed-off-by: astolat shalott [email protected]