draper icon indicating copy to clipboard operation
draper copied to clipboard

Work in progress fix for #538

Open brocktimus opened this issue 11 years ago • 6 comments

More details in #538. This is just extracted from a monkey patch I've used briefly in an application as discussed there.

This obviously needs testing but I'm unsure best / most appropriate way to go about it. It requires a lot more "stuff" than currently exists in the test suite (I think...?).

I also thought it should go into its own name space or something for other optimizations? I'm guessing something similar could be written for other AR relations and potentially Mongoid as well.

Any thoughts / advice appreciated!

brocktimus avatar Dec 01 '13 04:12 brocktimus

Could you rebase this, please? I've updated the travis file to actually build the correct rubies, and I'd like to see if it passes. Thanks!

steveklabnik avatar Jun 01 '14 06:06 steveklabnik

Done. I'm still not sure on best way to test this. Any ideas?

brocktimus avatar Jun 01 '14 14:06 brocktimus

Is this dead?

Having Draper blow away preloaded associations is a massive hit to performance.

stouset avatar Sep 22 '14 22:09 stouset

I'm still around, but haven't heard anything back since rebasing. Test fail on Rails 3 but I think we just skip that since it's super unsupported now. Still pretty unsure about testing however. i'll write a test for an idea I have now.

EDIT: Just looked and remembered the reason it was hard to test is at the moment the suite doesn't seem to have any models defined for associations and stuff. I'll have a bit of a look into the dummy app and see what I can do though, but any help would be greatly appreciated.

brocktimus avatar Sep 23 '14 00:09 brocktimus

Seems really interesting but we need test support / documentation.

jcasimir avatar Mar 27 '15 04:03 jcasimir

Based on #538 is this one also invalid if #decorate is being removed?

Otherwise I can look at it more just might need a bit of advice in best way of testing it. Most of the current testing stuff didn't seem to be dependent on having a full blown AR setup which this seems to start to require.

brocktimus avatar Mar 27 '15 11:03 brocktimus