search

vanstyn / RapidApp

Turnkey ajaxy webapps
http://rapi.io
Other
48 stars 15 forks source link

Multi-rel column counts ignore 'where' in relationship attr #95

Closed vanstyn closed 9 years ago

vanstyn commented 9 years ago

Regression which started with 0be555eac684 after the refactor to use CorrelateRelationship

vanstyn commented 9 years ago

According to @ribasushi this is a DBIC issue which is expected to be fixed in DBIx::Class 0.082900. I will defer to him to explain further...

In the meantime, a workaround is being written for RapidApp...

ribasushi commented 9 years ago

Deliberations on this issue are still ongoing. A big chunk of the work will likely resolve due to a conversation on Jan 31st. More updates in the first/second week of Feb.

vanstyn commented 9 years ago

Since code is working now as-is, I'm closing. We will revisit when DBIC changes are made, which at that point will be a code cleanup to make use of updated APIs (once they are available)

ribasushi commented 9 years ago

o/ @vanstyn I finally arrived at this spot from a different side, with more context to help me see how to actually tackle this. As an extra sanity check I need to recreate your original problem within rapidapp. Can you provide me with the exact commitish which exhibited the oh noes where doesn't work, and a pointer to a minimal test (which I hope you have somewhere in the repo) demonstrating a failure?

Cheers

vanstyn commented 9 years ago

It stopped working when I did the refactor in this commit (also listed in the body/first comment):

https://github.com/vanstyn/RapidApp/commit/0be555eac68471145d66dff194560df3761c7137

I switched to try to use 'correlate' to follow the more correct API, but that broke 'where' and I had to switch back (and silence the warning DBIC start throwing)

ribasushi commented 9 years ago

@vanstyn What I meant is - which test (as in <something>.t) can I run to see the but that broke 'where' part

vanstyn commented 9 years ago

Just closing the loop from IRC ... there isn't a RapidApp test case for this, but @ribasushi will send me a tarball with his prospective fix and I will manually test it