From b7155a5e35f85d32996154fbfd7c47e6d6115904 Mon Sep 17 00:00:00 2001 From: Geremia Taglialatela Date: Mon, 12 Aug 2024 09:39:25 +0200 Subject: [PATCH] Fix batch methods for historical models In the default implementation, `cursor` defaults to `primary_key`, which is 'id'. However, for historical models, we need to use 'hid' instead of 'id'. This patch addresses an issue where `with_hid_pkey` is called after the cursor is already set, potentially leading to incorrect behavior. Notes: - `find_each` and `find_in_batches` use `in_batches` internally, however this is not enough anymore in the upcoming Rails 8.0 because there is a conditional branch using `enum_for` - This approach prevents specifying 'id' as a cursor for historical models. If 'id' is needed, it must be handled separately. Fix #321 --- lib/chrono_model/patches/batches.rb | 36 ++++++++++++++++++++++++++++- 1 file changed, 35 insertions(+), 1 deletion(-) diff --git a/lib/chrono_model/patches/batches.rb b/lib/chrono_model/patches/batches.rb index debacbe..1acdb43 100644 --- a/lib/chrono_model/patches/batches.rb +++ b/lib/chrono_model/patches/batches.rb @@ -2,10 +2,44 @@ module ChronoModel module Patches + # Overrides the default batch methods for historical models + # + # In the default implementation, `cursor` defaults to `primary_key`, which is 'id'. + # However, historical models need to use 'hid' instead of 'id'. + # + # This patch addresses an issue where `with_hid_pkey` is called after the cursor + # is already set, potentially leading to incorrect behavior. + # + # Notes: + # - `find_each` and `find_in_batches` internally utilize `in_batches`. + # However, in the upcoming Rails 8.0, this implementation will be + # insufficient due to a new conditional branch using `enum_for`. + # - This approach prevents specifying 'id' as a cursor for historical models. + # If 'id' is needed, it must be handled separately. + # + # See: ifad/chronomodel#321 for more context module Batches - def in_batches(**) + def find_each(**options) return super unless try(:history?) + options[:cursor] = 'hid' if options[:cursor] == 'id' + + with_hid_pkey { super } + end + + def find_in_batches(**options) + return super unless try(:history?) + + options[:cursor] = 'hid' if options[:cursor] == 'id' + + with_hid_pkey { super } + end + + def in_batches(**options) + return super unless try(:history?) + + options[:cursor] = 'hid' if options[:cursor] == 'id' + with_hid_pkey { super } end end