Multi-level counter-cache doesn't work with Discard
Opened this issue · 4 comments
It seems that couter_culture checks Discard of only one level of models hierarchy, but not all of them.
For example:
class Step < ActiveRecord::Base
include Discard::Model
belongs_to :lesson, class_name: 'Lesson'
counter_culture %i[lesson unit]
counter_culture :lesson
end
class Lesson < ActiveRecord::Base
include Discard::Model
belongs_to :unit
has_many :steps
end
class Unit < ActiveRecord::Base
include Discard::Model
has_many :lessons
has_many :steps, through: :lessons
end
u = Unit.create
l1 = u.lessons.create
l2 = u.lessons.create
l1.steps.create
l2.steps.create
u.reload.steps_count # = 2, correct
l1.discard
u.reload.steps_count # = 2, wrong, but okay
Step.counter_culture_fix_counts
## Generated SQL request, 'steps.discarded_at IS NULL' is checked, but 'lessons.discarded_at IS NULL' is not
# Unit Load (3.1ms) SELECT "units".id, "units".id, COUNT("steps".id)*1 AS count, MAX("units".steps_count) AS steps_count FROM "units" LEFT JOIN "lessons" AS lessons ON "units".id = lessons.unit_id LEFT JOIN "steps" AS steps ON "lessons".id = steps.lesson_id AND steps.discarded_at IS NULL GROUP BY "units"."id" ORDER BY "units"."id" ASC LIMIT $1 [["LIMIT", 1000]]
# Lesson Load (1.3ms) SELECT "lessons".id, "lessons".id, COUNT("steps".id)*1 AS count, MAX("lessons".steps_count) AS steps_count FROM "lessons" LEFT JOIN "steps" AS steps ON "lessons".id = steps.lesson_id AND steps.discarded_at IS NULL GROUP BY "lessons"."id" ORDER BY "lessons"."id" ASC LIMIT $1 [["LIMIT", 1000]]
u.reload.steps_count # = 2, still wrong
Expected:
Step.counter_culture_fix_counts
also takes into account discarded_at of parent model.
Actual:
Step.counter_culture_fix_counts
checks only discarded_at of Step, but any model on hierarchy.
For now we mitigated the issue with custom scope:
class Step < ActiveRecord::Base
include Discard::Model
belongs_to :lesson, class_name: 'Lesson'
scope :within_kept, -> { kept.where(lesson_id: Lesson.kept.select(:id)) }
counter_culture %i[lesson unit], column_name: ->(_) { 'steps_count' },
column_names: {
Step.within_kept => :steps_count
}
counter_culture :lesson
end
This sounds right—do you think you can put a pull request together to fix this? I don't use discard so wouldn't get around to fixing this myself.
Was this ever merged? Or do I use the above solution?
Nobody ever put together a PR to fix this, so there was nothing to merge. I don't have the time to work on this myself. If you can put together a PR for this I'd be happy to merge it!