Why this module requires views to be based on Commerce Order?
Why it requires Commerce Order ID as argument for view?
It would be more usable to allow to create a views of any type and any content (some information about services, suggestions to buy other related products, etc).

Comments

recrit’s picture

It really doesn't. The initial thought was to use it to replace some of the order based displays during checkout.
I can work up a patch that would remove the dependency for the commerce_order table as a base and allow any arguments and only pass the order id to the view if the first argument is order id.

berdyshev’s picture

Status: Active » Needs review
StatusFileSize
new3.25 KB

Here is a try to implement that.

recrit’s picture

Thanks for the patch! I took the same idea in your patch a little bit further.
The attached "...-3.patch" has 2 new functions for:

  • Get the position of all order id arguments
  • Build the arguments array based on the order id positions

Then the checkout pane uses the build arguments function for the array that it passes to the views execute function.
The patch also includes the updated README instructions.

I've tested this with a node view as a checkout pane, and it rendered correctly.
Test it out and then I can get it committed.

berdyshev’s picture

Thanks, it works perfect.

If you allow users to have more than one argument, lets allow them to configure value for all arguments on pane configuration page.
I have added fields for each argument of the view and list of replacement patterns from the current Commerce Order entity.

Please, review and give your feedback.

recrit’s picture

I like the direction as it would make it more flexible instead of always sending order_id in those argument positions.

Some items that need updated / consideration:

  • The settings variable name in your patch made me realize that the pane id should be prepended with "commerce_views_pane", ie module file, line 65 should be
    $panes['commerce_views_pane-' . $result['pane_id']] = $result + $defaults;
    

    Then all the splitting would need updated to account for it. This would make the pane and settings variables unique this module.

  • The key of the arguments array returned by $view->get_items('argument', $display_id) is the views argument id, not the field name, which is why I had a check for $arg_data['field'] == 'order_id'. The arg id could be order_id, order_id_1, etc if there are more than one. So in the settings form, it would check if the 'field' was order_id to provide the suggested default.
  • _commerce_views_pane_view_build_order_arguments()
    • For the arguments array to match what views would be expecting, it should always add a NULL to $return if !isset($arg_data['field']) AND nothing is being added by the settings. There could be an argument that doesn't have a field, perhaps view global null or something.

I'll open up a separate issue for the pane id changes. For this patch, we can focus on making the settings variable building and splitting account for it.

recrit’s picture

pane id creating and parsing fixed per #1952772: Pane id should be unique to this module. The patch in this issue will need to use the helper functions for the pane id creating and parsing.

berdyshev’s picture

Thanks for feedback. I've fixed your comments.
For the variable name I use underscore as separator, because generally the dash isn't used for that purpose.
The patch re-rolled against the HEAD.

berdyshev’s picture

Sorry, wrong patch in previous comment.
Here is the correct

recrit’s picture

Status: Needs review » Fixed

The variable name with underscore would be ok since we're never splitting it apart to extract the view id or display id which are allowed to have underscores, hence why the pane id is "-".

Thanks, I've committed per http://drupalcode.org/project/commerce_views_pane.git/commit/9afb65a

berdyshev’s picture

Status: Fixed » Active

You need also to update module description according the new changes :-)

recrit’s picture

Status: Active » Fixed

nice catch, I updated it to match the README

Status: Fixed » Closed (fixed)

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

matthand’s picture

It's not clear to me how to set the Order ID argument now. I actually liked the module more before this change!

So, since Arguments = Contextual Filters in Drupal 7 Views UI... How can I set the Order ID Contextual Filter? Does it only offer it on views where the content relates to orders?

Thanks for the help!