search

brendon / acts_as_list

An ActiveRecord plugin for managing lists.
http://brendon.github.io/acts_as_list/
MIT License
2.04k stars 355 forks source link

Ignoring STI #354

Closed corwinstephen closed 5 years ago

corwinstephen commented 5 years ago

I've been using a forked version of this gem for a couple years now that I created to support the case where ordering needs to be scoped to a base class rather than an STI subclass, ie I have a bunch of "steps" that themselves have "types", but the steps still need to be sorted globally, not within their respective types.

Given that every now and again I have to keep going back and merging this gem into my forked one, I figured I'd at least ask to see if anyone else is interested in incorporating this feature?

Ex: https://github.com/corwinstephen/acts_as_list/blob/dc16d90dd29e88f40104ccee3d32eba4112c4d18/lib/acts_as_list/active_record/acts/list.rb#L35

brendon commented 5 years ago

Hi @corwinstephen, I'm always keen for improvements to the gem :)

I'm struggling to understand your use case. If you called acts_as_list scope: [:some_parent_association_id] on the base class, that should ignore STI classes when manipulating the list. Is that not the case for you?

Perhaps do up a PR showing the differences and we can go from there :)

corwinstephen commented 5 years ago

@brendon thanks for the quick response!

Did this behavior change at any point? At the time of my original forking (3-4 years ago) I had what you've suggested:

class Step
    acts_as_list scope: :template
end

class StepTypeA; end
class StepTypeB; end
class StepTypeC; end
# Etc etc...

In that case, when I would update the position of a step:

step = Step.find(params[:id])
step.update(position: 2)

it would scope the query for the total number of steps to the specific step type, and ignore the other types, producing incorrect numbering.

brendon commented 5 years ago

@corwinstephen, I can't quite remember but I think we did some work around unscoping and STI. It's Rails that's adding the extra STI scope from what I remember. Perhaps give the latest version a try without your modifications and see if it's still an issue.

Possibly related:

corwinstephen commented 5 years ago

@brendon It looks like #147 is probably related, and you're correct, I just tried it with the latest and it seems like it's no longer an issue! I should have checked before I opened this. Sorry about that!

Thanks!

brendon commented 5 years ago

No worries at all. I'm always happy for these threads as it'll help someone else searching the same problem :)