Hi,

Ì converted the query that gets all the combinations for the adjustments form to a dynamic query.
This way i can alter the query from a contrib module i just created on http://drupal.org/project/uc_attribute_ignore

This patch just does exactly the same as the original query, i just added a tag to it.
Could this be committed?

Cheers!
-- Tom

CommentFileSizeAuthor
#4 1828596-attribute-adjustments-dynamic-query-4.patch1.86 KBlongwave
#2 1828596-attribute-adjustments-dynamic-query.patch1.83 KBlongwave
ubercart-attribute-query.patch1.83 KBbetz

Comments

betz’s picture

betz commented
Status: Active » Needs review
longwave’s picture

longwave
he/him
Primary language English
Location UK
commented
StatusFileSize
new 1828596-attribute-adjustments-dynamic-query.patch1.83 KB

I fixed the coding standard issues in the patch, but otherwise this is unchanged. We have a test for the adjustments form, so if this passes and you confirm it's okay, then I will commit it.

Status: Needs review » Needs work

The last submitted patch, 1828596-attribute-adjustments-dynamic-query.patch, failed testing.

longwave’s picture

longwave
he/him
Primary language English
Location UK
commented
Status: Needs work » Needs review
StatusFileSize
new 1828596-attribute-adjustments-dynamic-query-4.patch1.86 KB

Heh, well at least the tests work.

betz’s picture

betz commented
Status: Needs review » Needs work

Yes, I could't get this query to work in this way also.
So I guess it is more then just coding standards, it changes the query all together...

longwave’s picture

longwave
he/him
Primary language English
Location UK
commented
Status: Needs review » Needs work

leftJoin() is not chainable, as it returns an alias. #4 seems to work, though.

Chainable vs non-chainable methods: http://drupal.org/node/1060924

longwave’s picture

longwave
he/him
Primary language English
Location UK
commented
Status: Needs work » Needs review
betz’s picture

betz commented
Status: Needs work » Reviewed & tested by the community

Just applied patch to dev and tested, everything seems to work normally.

longwave’s picture

longwave
he/him
Primary language English
Location UK
commented
Status: Reviewed & tested by the community » Fixed

Committed.

Status: Fixed » Closed (fixed)

Automatically closed -- issue fixed for 2 weeks with no activity.