Madmin sortable helper conflicts with JSP sortable helper

[timmyd87]

Hi Team, thought I'd have another go at installing Madmin and it's installed without a hitch on JSP this time (yay!!), only conflict I'm seeing is my main app views is now looking at the Madmin sortable helper and throwing a 'wrong number of arguments (given 4, expected 2..3)' error.

The JSP sortable helper includes a 'relation' attribute in addition to the 'column, title, options = {}'.

What do you think is the best fix?

[jacobdaddario]

From what I remember, Rails dumps all of the helpers into one namespace by default when loading them for views. This might be why you're getting this collision. This forum thread recommends disabling loading all helpers by default. That would be a solution on the JSP side of things. Let me see if I can find something to isolate those Madmin helpers from your main app.

Edit: Ah, looks like Rails Guides recommends using isolate_namespace specifically citing helper name collisions. Looks like Madmin doesn't do this. This could be an easy change but I'm not sure if this would cause more problems. Give me a second to read some more on this and then I'll try and submit a PR if it looks like it won't have tons of side-effects.

Final Edit: So just chucking isolate_namespace in there is causing the test suite to fail. My suspicion is and was that isolate_namespace was removed in order to achieve some kind of functionality for Madmin. I'll try and investigate further to see if I can find a way to just isolate helpers. I doubt Madmin helpers would need to bleed into the main app. I may take a week or so to do this. I'm not sure what my availability will be like after this morning.

[jacobdaddario]

Submitted a PR for this!

[timmyd87]

Thanks so much for taking a look at this and explaining the root cause! It's interesting to know rails loads all of the helpers into the one namespace by default, that's something I didn't know but is very helpful. Also, thank you for turning around a PR for this so quickly, very unexpected.

[jacobdaddario]

I had some time this morning and was interested to see if I could pull it off. It's a bit of a hacky solution so hopefully it's acceptable to merge. You definitely may need to double-check me on the helpers being imported in every context. I'm pretty sure that they are though.

[excid3]

I don't remember why, but isolate_namespace was removed in https://github.com/excid3/madmin/commit/79977234dc46c42d61e510506d4e62de1a5ddbda#diff-80e180dc8e007fcf0b20fcde44ed2681b6d72390e6f8e07acc216be9afd3659bL3

I don't think there's any reason we can't reintroduce it. In general, the admin area is isolated anyways. We should see about adding that back in.

[jacobdaddario]

When I tried dropping it back in it caused something in the test suite to fail. I figured it had to do with why it was removed in the first place. I can try experimenting more to see what happens

[excid3]

Let me know what the error is if you try it again. 👍

[jacobdaddario]

Taking a closer look at it. Weirdly, it causes a test to check that the users resource route exists to fail.

[excid3]

I am not entirely sure why url_for doesn't work with isolate_namespace. Our code seems to match Administrate here and it works for that gem. Maybe someone more familiar with the router would know.

In the meantime, using the resource path helpers works with isolate_namespace: b3d2a0f