Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Can I Create a PR with a Function that "Fixes" List in Case of Duplicates? #333

Closed
pacarvalho opened this issue Dec 18, 2018 · 6 comments
Closed

Comments

@pacarvalho
Copy link

pacarvalho commented Dec 18, 2018

I have noticed that under some peculiar situations it is possible to end up with a list that contains duplicate positions in acts_at_list. The duplicate positions cause unexpected behaviors to acts_as_list and sometimes leads to errors.

Would it be OK if I created a PR with a function that fixes lists that contain duplicate positions?

It could be done by arbitrarily choosing an order for the items with duplicated positions (there is no way of knowing which should come first with respect to one another) and moving other items out of the way.

Related to #76 #317

@brendon
Copy link
Owner

brendon commented Dec 18, 2018

Hi @pacarvalho, yes that's a known fault and usually occurs due to simultaneous requests that manipulate the list. I'd be very keen for you to come up with a way to self-heal lists like these that isn't too too expensive to run. If you have a look through some of the issues on this repo you'll see there's been attempts to implement various locking mechanisms. The latest involved an advisory lock but it turned out it needed more restructuring to the gem than the author was prepared to do at the time, but you're more than welcome to continue his work :)

@DanielHeath
Copy link

I've got an efficient solution for this for postgres (using window functions) and mysql (using connection variables), but sqlite lacks a nice way to do this.

DanielHeath added a commit to DanielHeath/acts_as_list that referenced this issue May 30, 2019
@brendon
Copy link
Owner

brendon commented Jun 3, 2019

Hi @DanielHeath, this looks promising. I see it in your PR :) The only thing I can think of is that it'd be nice to tidy up these code forks for specific database backends into seperate files so they can be maintained more easily?

@DanielHeath
Copy link

Happy to split it up, but would appreciate some guidance about what shape it should take - I tend to favor "too much code" rather than "where's the code", when making that sort of judgement call so I'm less familiar with good patterns for this kind of split.

@brendon
Copy link
Owner

brendon commented Jun 3, 2019

Hehe, yea, I'm not 100% sure I like the current project structure with all those definers. I like the way ancestry structures their code. But that's a project for another day.

I'm thinking perhaps when loading methods into the model, we could do a check for the current database adapter (on the model - as some projects will have different databases on different models), we then include a file based on whether it's sqlite, mysql or postgres that have identical methods with different implementations?

@brendon
Copy link
Owner

brendon commented Jun 4, 2024

Closing this for now. Let me know if you want to look at it again. Check out https://github.com/brendon/positioning as an alternative to acts_as_list that uses an advisory lock to prevent corruption.

@brendon brendon closed this as completed Jun 4, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging a pull request may close this issue.

3 participants