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

Multiple action forms on one page leads to unpredictable results #659

Closed
alanmcruickshank opened this issue Dec 5, 2017 · 2 comments
Closed

Comments

@alanmcruickshank
Copy link

This references an issue we found as part of adding a feature to superset.

Issue

Using the CompactCRUDMixin and a tabbed view we have a page for one model, which contains several cruds for a selection of dependent models. We'd like to enable bulk actions on each of these dependent cruds.

The issue we're facing is that the tabbed interface means that each of the cruds is present on the page (although some are hidden) all the time. Bulk actions are implemented using the AdminActions class (defined here ) and the button is rendered in base_list.html using the action_form method of lib.html. On clicking the button we eventually use the selector #action_form for the AdminActions class to find the hidden form it needs to submit.

The issue we're facing is that we have multiple cruds on one page, which means multiple forms which can be matched with the selector #action_form, and in out case that sometimes means submitting the form for a model which wasn't intended (and deleting the wrong records).

Proposed Fix

I'd like to make the id for the hidden forms more unique rather than just #action_form. The simplest way I can think of doing that would be to add the name of the model to the end of the id. For example if my model was TableColumnInlineView then the rendered form would use the id action_form_TableColumnInlineView, which means when we call form_submit in ab_actions.js it would use the selector #action_form_TableColumnInlineView and so uniquely identify the relevant form to submit.

Does this sound like a sensible fix? I don't do a ton of javascript work so I'd love to hear any more elegant solutions to the problem. I'm happy to do the legwork and submit a PR in due course.

@dpgaspar
Copy link
Owner

dpgaspar commented Dec 5, 2017

Yes, this sounds like a sensible fix. A PR with this would be great

@dpgaspar
Copy link
Owner

dpgaspar commented Jan 6, 2018

Closing this one.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

2 participants