Problem/Motivation

Although both the view_name and display and being stored on the main draggableviews table, when the join is performed and used as a sort field, only the entity_id is used to join the tables. If different draggableviews have the same node, it will be displayed more than once on all views that has the draggable view sort handler.

Proposed resolution

Add dropdown to the draggableviews weight sort for users to select which view and which view display to user for sorting.

Remaining tasks

create update_hook() to load the views with a draggableviews weight sort -> add the current view (display X) to all displays with that view name -> then resave them. We did something like this in D7, but was never committed. https://www.drupal.org/node/2413433#comment-9823751 #2413433: update 'self' sort handler setting in db

CommentFileSizeAuthor
#117 draggableviews-sort_handler_specify_order_view-2767437-116.patch10.36 KBwmcmillian-coalmarch
#116 draggableviews-sort_handler_specify_order_view-2767437-116.patch0 byteswmcmillian-coalmarch
#78 draggableviews-sort_handler_specify_order_view-2767437-78.patch8.25 KBdevad
#70 draggableviews-sort_handler_specify_order_view-2767437-70.patch76.49 KBAdrian83
#66 draggableviews-sort_handler_specify_order_view-2767437-66.patch76.46 KBahebrank
#65 2767437-n65.patch8.11 KBhanoii
#64 draggableviews-sort_handler_specify_order_view-2767437-64.patch75.39 KBpingevt
#63 draggableviews-sort_handler_specify_order_view-2767437-63.patch75.39 KBpingevt
#60 interdiff-59-60.txt786 byteskerasai
#60 draggableviews-sort_handler_specify_order_view-2767437-60.patch6.34 KBkerasai
#59 interdiff-58-59.txt1.13 KBkerasai
#59 draggableviews-sort_handler_specify_order_view-2767437-59.patch6.35 KBkerasai
#58 sort-2767437-58.patch6.21 KBpingevt
#56 sort-2767437-56.patch5.83 KBpingevt
#53 3-Click_on_error-draggableviews.log_.txt30.25 KBfrederickjh
#53 2-Add__and _configure_relationships_error-draggableviews.log_.txt194.14 KBfrederickjh
#53 1-Add_relationship_error-draggableviews.log_.txt96.73 KBfrederickjh
#49 2767437-48.patch8.04 KBhanoii
#44 draggableviews-sort_handler_select_display-2767437-44.patch7.41 KBFernly
#38 2767437-n38-remove-update-hook-for-now.patch1.75 KBhanoii
#37 2767437-36.patch8.04 KBhanoii
#32 2767437-n32.patch7.9 KBiStryker
#31 2767437-n30.patch3.84 KBiStryker
#29 sorting_source-draggableviews-2767437-29.patch4.38 KBiStryker
#28 update_hook-draggableviews-2767437-27.patch1.91 KBiStryker
#23 2767437-n23.patch7.88 KBhanoii
#22 2767437-n22.patch3.82 KBhanoii
#14 2767437-n14.patch4.23 KBhanoii
#11 2767437-n11.patch4.23 KBhanoii
#9 2767437-draggableviews-d7.png30.06 KBiStryker
#9 2767437-should_see_form.png58.85 KBiStryker
#8 interdiff-2767437-6-8.txt1.61 KBhanoii
#8 2767437-n8.patch2.97 KBhanoii
#6 2767437-n6.patch2.4 KBhanoii
#2 draggableviews-filter_for_source_view-2767437.patch2.06 KBivan.prokopenko
#79 draggableviews-sort_handler_specify_order_view-2767437-79.patch76.41 KBdevad
#81 draggableviews-sort_handler_specify_order_view-2767437-81.patch76.35 KBdevad
Command icon Show commands

Start within a Git clone of the project using the version control instructions.

Or, if you do not have SSH keys set up on git.drupalcode.org:

Support from Acquia helps fund testing for Drupal Acquia logo

Comments

ivan.prokopenko created an issue. See original summary.

ivan.prokopenko’s picture

ivan.prokopenko’s picture

mroest’s picture

This patch did not work for me as the filter requires that the weight is set for every record/entity

iStryker’s picture

Title: Filter to make view use source view » Sort filter to make view use source view
Issue summary: View changes
Status: Active » Needs work
Parent issue: » #2906059: Plan for next stable Draggable Views release

This is a great start. It needs to be added to Sort Handler.

Adding this to 8.x-1.1 release

hanoii’s picture

Title: Sort filter to make view use source view » Allow sort handler to select the view that stored the order
Issue summary: View changes
Status: Needs work » Needs review
FileSize
2.4 KB

Here's an attempt for this patch.

This is an odd one to tackle and I can't remember how this was handled on D7.

This adds the view selection to a newly created sort handler. It also modified the query by altering the join (if it would be where, it could render and empty field on new additions).

It also assumes some things.

I think it's a good start, and good for an initial review.

An addition to this could be to also select the display of the view, which is another column on the draggableviews_structure table.

hanoii’s picture

hanoii’s picture

FileSize
2.97 KB
1.61 KB

Added some support for view_display. It's a starting point, it can at least be typed now.

iStryker’s picture

Status: Needs review » Needs work
FileSize
58.85 KB
30.06 KB

The code in the patch looks sound, but then again I never tried something like this before.

2 things.
#1 - I cannot get the form to display. I tried the Draggableviews demo view (separate module) and I cannot get the form to display on the 'display page' view display.

where I expect the form to be

#2 - With Drupal 7 we combined the view page and view display together into one dropdown. This would be a nice feature to have instead of a textfield.

What the sort handler looks like in Drupal 7

hanoii’s picture

for #1.. it's odd, I see it fine even on the demo submodule.

Maybe clearing cache so that the new views data is picked up? making sure views cache is cleared.

As for #2, yes, I agree. This was at least something initial and even something I can use on a project I need this. I was thinking in doing something like what you said. The odd part is going over the actual view when the details is not on the config. Not super hard, just couldn't get to it yet.

hanoii’s picture

FileSize
4.23 KB

Here's another attempt, with a similar functionality to #2.

hanoii’s picture

Status: Needs work » Needs review
iStryker’s picture

Patch 11 worked for me. This time tested locally with a fresh installation and with simpletest.me.

I did a simple test across multiple views and it worked, which I wasn't expecting. That is awesome.

On fresh install on simpletest.me the draggableviews demo display works. The order page does not. Not until you select the order view display. All Draggableviews weight should be 0/null so it default the sort to author on.

Next Step. The order view should default to to itself. The display view should default to the first order view. With this hopefully, we do not need to do any extra work when people upgrade from 8.x-1.0 to 8.x-1.1, as it will pick on the default settings.

hanoii’s picture

FileSize
4.23 KB

Rebased the patch against latest dev and tweak the default.

@iStryker this could help with existing sites. Adding a more sensible default could be trickier. The way I thought it is that if someone is adding a draggableviews sort, just force him to chose the view from the existing one. But having the current view on top.

Otherwise I am not sure I understood your Next steps.

iStryker’s picture

Issue summary: View changes
Status: Needs review » Needs work

Looks good

So patch #14 fixes #2796313: Duplicate entities when 2 different entity types have the same entity_id because of

  public function query() {
    parent::query();
    $table = $this->query->getTableInfo($this->tableAlias);
    if (!empty($this->options['source']) && $table) {
      list($view_name, $view_display) = explode('|', $this->options['source']);
      $table['join']->extra = [['field' => 'view_name', 'value' => $view_name]];
      $table['join']->extra[] = ['field' => 'view_display', 'value' => $view_display];
    }
  }
$table['join']->extra = [['field' => 'view_name', 'value' => $view_name]]; 

should be

$table['join']->extra[] = ['field' => 'view_name', 'value' => $view_name];

as it might override another extra join from someones custom code.

This issue now needs an update_hook() to load the views with a draggableviews weight sort -> add the current view (display X) to all displays with that view name -> then resave them. We did something like this in D7, but was never committed. https://www.drupal.org/node/2413433#comment-9823751 #2413433: update 'self' sort handler setting in db

iStryker’s picture

Issue summary: View changes
hanoii’s picture

should be

$table['join']->extra[] = ['field' => 'view_name', 'value' => $view_name];

Ok, I will change this, but will have to be an if I think, because if it's not an array it might fail.

This issue now needs an update_hook() to load the views with a draggableviews weight sort -> add the current view (display X) to all displays with that view name -> then resave them. We did something like this in D7, but was never committed. https://www.drupal.org/node/2413433#comment-9823751 #2413433: update 'self' sort handler setting in db

I am hesitant on this. We are doing assumptions here that although could normally be the case, it's not 100% accurate. I believe not doing anything here is OK, as it would allow the current views to simply work as they were before. With this, if someone is OK using it how it is (because it uses draggable views on one single view and then this addition is unnecessary to them) it will still work for them. If they had an issue, they can go and fix it when they upgrade.

iStryker’s picture

I think the assumption is fine. Currently there is no other way this works unless you have 1 view with 1 display and 1 order. (or 1 order and X displays). If we do not have an update hook then problems like #2796313: Duplicate entities when 2 different entity types have the same entity_id will no be fixed.

iStryker’s picture

Yeah I was wondering about if its not an array, maybe do a check, or check if documentation if its always an array

iStryker’s picture

Here is the work I have done so far. Want to post it here as I am going MIA for a week or so, and want to capture what I have done so far.

Logic
if it has the draggableviews field and is NOT default then thats the order display.
- Find all sort[weight] and set source to the view

if it has the draggableviews field and is default then we need to find (A) fields that are NOT false, then they are the order display. or (B)
-

Most likely sort weight will be default as it will be used for all views.

Code

    foreach (\Drupal::entityTypeManager()->getStorage('view')->loadMultiple() as $view) {
        $views_displays = $view->get('display');
        foreach ($views_displays as $views_display) {
            if ($field = $views_display['display_options']['fields']['draggableviews'] ?? NULL) {
                // This display is an order view.
                if ($views_display['display_plugin'] == 'default') {

                }
            }
            if ($field = $views_display['display_options']['sorts']['weight']['field'] ?? NULL) {
               if ($field = 'weight') {
                   // This display has the sort weight
                   $test = '1';
               }
            }
        }
hanoii’s picture

#2796313: Duplicate entities when 2 different entity types have the same entity_id is probably an issue that has to be sorted besides this but not by SQL, but rather storing the entity type on the table and then doing a proper join with it.

But we can go back to that later.

Also, I am completely reworking this bit.. because I needed to be more flexible in terms of using two draggableviews sorting on the same view, and this approach didn't allow me to do that.

I will be using relationships for this.

I will come back with a new patch and we can re-discuss a possible hook_update().

hanoii’s picture

FileSize
3.82 KB

OK, this is what I come up with.

I removed the implicit join relationship, as that was going causing issues and now "requiring" a relationship. The relationship allows you to select the views display you want to use as per the above patch.

With this approach, you can now add as many sort orders as you want and they will still work.

Now this patch could break existing sites so an update hook could be more needed. But not sure if it's gonna be easy.

But I really need this so adding it here for further discussion/consideration.

hanoii’s picture

FileSize
7.88 KB

Proper patch with the new relationshiphandler.

iStryker’s picture

Patch looks great

iStryker’s picture

Can you tell me what did not work with #14 as I prefer to keep it all in the sort and not create an extra step of creating a relationship.

hanoii’s picture

I have an use case where we have two featured groups of content, each feature group is a draggableviews admin table where the sorting happens.

I then need to display content where the first featured group comes first, and the second comes next, and within each featured group, the draggableviews order.

For this to work, I need to order by draggableviews_1 and then draggableviews_2. This is not possible to do if the join comes from the sort handler, unless we complicate it further and add some way of configuring this there. The the relationship makes the patch super simple.

A middle term would be providing both things and allow the user to decide what to use but I honestly prefer the relationship.

iStryker’s picture

Working up the update_hook(). I cannot find the command to set the value so I can save the view ($view->save). I have ask the question on stackexchange. https://drupal.stackexchange.com/questions/248432/8-how-do-a-get-a-view-...

Here is what I got so far


$views = Views::getEnabledViews();
foreach ($views as $view) {
  $source_value = '';
  $config = \Drupal::config('views.view.' . $view->id());
  $rawData = $config->getRawData();

  $master = FALSE;
  // Get the display with the draggableviews field.
  // It is a little tricky because the 'default' display is the master.
  foreach ($rawData['display'] as $display_key => $display) {
    if ($display_key == 'default' && array_key_exists('draggableviews', $display['display_options']['fields'])) {
      $master = TRUE;
    }
    if ($display_key != 'default' && array_key_exists('draggableviews', $display['display_options']['fields'])) {
      $source_value = $view->id() . '|' . $display_key;
    }
    if ($master && empty($display['display_options']['fields'])) {
      $source_value = $view->id() . '|' . $display_key;
    }
  }
  if ($source_value) {
    $master = FALSE;
    // Save the view with the source value in the sort handler.
    // It is a little tricky because the 'default' display is the master.
    foreach ($rawData['display'] as $display_key => $display) {
      if ($display_key == 'default' && array_key_exists('draggableviews_structure', $display['display_options']['sorts'])) {
        $master = TRUE;
      }
      if ($display_key != 'default' && array_key_exists('draggableviews_structure', $display['display_options']['sorts'])) {
        $display['display_options']['sorts']['weight']['source'] = $source_value;
        // RESAVE THE VIEW HERE
      }
      if ($master && empty($display['display_options']['fields'])) {
        $display['display_options']['sorts']['weight']['source'] = $source_value;
        // RESAVE THE VIEW HERE 

      }
    }
  }

Can exchange the sort for the relationship depending which way we go.

iStryker’s picture

Status: Needs work » Needs review
FileSize
1.91 KB

Got the update hook for adding it to the sort. Patch attached of just the update_hook()

iStryker’s picture

Actually I'll upload the sort handler patch. Contains coding standards and join handler fix

hanoii’s picture

Does this mean that the relationship handler is not going to be accepted?

iStryker’s picture

FileSize
3.84 KB

@hanoii maybe. I wanted see if I could get the update hook working with the sort handler. I knew if I got it then rewriting it would be easy for relationships.

Re-rolling n23 so I can upload it to simpletest.me and get UI feedback from co-workers. I think it comes down UI preference.

Cons
- One extra step
- People will not be use to it if using 8.x-1.0 or 7.x-2.x. Need to highlight.

Pros
In 7.x-2.x there was a large amount of annoying support issues (I'm guess 15-25) about not selecting the correct source or not seeing it. When you first create the view and add the field, the source items do not show up (like image of sort handler without source). You have to save, then they show up (like of the source showing up in sort handler). This was extremely confusing because the sort would work, but there would be no source dropdown in the Views UI. If there is an extra step of adding a relationship then maybe people will not stumble on this.

iStryker’s picture

FileSize
7.9 KB

Forgot relationship handler in patch n30. Re-rolled with coding standards.

iStryker’s picture

I spoke with co-workers and going with the sort is simpler and is the method I would like to go with

hanoii’s picture

Would you consider adding both things?

iStryker’s picture

Maybe, I'm trying to figure out why you need relationships. I tried 2 sorts in one view without relationships and it didn't work as it did a join on view_name = 'ViewName1' and view_display = 'ViewDisplay1' and view_name = 'ViewName2' and view_display = 'ViewDisplay1'....which clearly will not work. I tried 2 relationships and it threw an error on simplytest.me. I have yet to have time to troubleshoot this.

FYI in both cases I install draggableviews_demo, then duplicate the draggableviews_demo view -> add the non-duplication source then the duplication/current view source.

hanoii’s picture

Re-rolling #23 for now.

I am trying to pick this up and move it forward.

I managed to come up with a sort handler that would allow for multiple sort handlers within the same view, however, its yet not as flexible as relationship.

I know you are thinking for the majority of use cases, but by making this a relationship, you just adds a tiny complexity that can be properly documented and you gain a lot of flexibility.

I can think of other use cases, yet odd, that even with my improved sort handler won't work.

i.e.:

Say that you have Content type A with an entity reference for Content type B

And you have a draggableviews admin screen of content type B where you order.

And then you want a view that shows all Content type A sorted by the draggableviews sorted of content type B.

In pseudo SQL would be something like:

Select * FROM content_type_a
left join content type_b on content_type_a.referenceid = content_type_b.id
left join draggableviews_structure on entity_id = content_type_b.id and views_name = 'some' and display = 'other'
order by draggableviews_structure.weight

With a relationship this comes out of the box.

Now, if you are set on having the sort handler, a middle point solution would be to add both things, and if a relationship is selected on the sort handler, it won't use/display the configurations below. Ideally we wouldh ave to have some custom validations in order to show/hide the join handlers on the view and only make it required if a relationship is not set on the sort handler.

I guess this might not be too much work either and maybe if you are more happy with the sort handler this will have the best of both things.

hanoii’s picture

FileSize
8.04 KB

Forgot re-roll

hanoii’s picture

Hey.. I think you accidentally committed the update hook on http://cgit.drupalcode.org/draggableviews/commit/?id=b8ba4d2653bbef48604...

Adding a patch to remove this so I can plug this into my composer, but I'd recommend removing it until we wrap this up.

iStryker’s picture

Yes, Looks like I committed it by accident.

acrosman’s picture

Status: Needs review » Needs work

I tried the patch from #37 and for a preexisting view that cause views to generate a SQL error.

The website encountered an unexpected error. Please try again later.</br></br><em class="placeholder">Drupal\Core\Database\DatabaseExceptionWrapper</em>: Exception in Board of Directors List[board_of_directors_list]: SQLSTATE[42S22]: Column not found: 1054 Unknown column &#039;weight&#039; in &#039;field list&#039;: SELECT node_field_data.title AS node_field_data_title, node_field_data.nid AS nid, weight AS weight
FROM 
{node_field_data} node_field_data
INNER JOIN {node__field_categories} node__field_categories ON node_field_data.nid = node__field_categories.entity_id AND node__field_categories.deleted = :views_join_condition_0
WHERE ((node__field_categories.field_categories_target_id = :node__field_categories_field_categories_target_id)) AND ((node_field_data.status = :db_condition_placeholder_1) AND (node_field_data.type IN (:db_condition_placeholder_2)))
ORDER BY weight ASC, node_field_data_title ASC; Array
(
    [:node__field_categories_field_categories_target_id] =&gt; 1
    [:db_condition_placeholder_1] =&gt; 1
    [:db_condition_placeholder_2] =&gt; person_detail
    [:views_join_condition_0] =&gt; 0
)
 in <em class="placeholder">Drupal\views\Plugin\views\query\Sql-&gt;execute()</em> (line <em class="placeholder">1528</em> of <em class="placeholder">core/modules/views/src/Plugin/views/query/Sql.php</em>).
4kant’s picture

I successfully applied the patch #37.
The errors about the missing weight-column has gone away after using relationship to draggableviews content in all views-displays that have a draggableviews order.
Actually I don´t have taxonomy in use of the respective views.

Thanks so far

acrosman’s picture

In a second attempt I did get #37 to work (a couple extra cache clears and I could add the relationship and get it run). Re-reading all the back comments, it looks like it's still unclear the best way to resolve this issue and the right patch to be applying and testing. A little guidance on the best way to help out with testing and getting this to a resolution would be useful.

mpp’s picture

The patch in #37 doesn't apply to 1.0.0, would be nice if there'd be a new release from dev.

It looks better than the one in https://www.drupal.org/project/draggableviews/issues/2796313

Fernly’s picture

Status: Needs work » Needs review
FileSize
7.41 KB

Reworked the patch in #37 so it applies to 1.0.0.

Make sure you add the necessary views relationship 'draggableviews' to all views using draggableviews sorting. Otherwise you get the SQL error saying the 'weight' column is unknown.

Tested and works perfectly.

mpp’s picture

The patch in #44 no longer applies (since 1.1).

hanoii’s picture

@mpp you likely need to be using #38 instead.

hanoii’s picture

@iStryker I am removing the update for now as it cropped to the new release, I don't think ti should be there until we decide/complete this.

  • hanoii committed 8a12b68 on 2767437-sort-handler
    Adding sort handler as per [#2767437-37]
    
hanoii’s picture

Adding a re-rolled patch to latest dev now I updated the hook and commit it to a feature branch.

This will work against current dev and at least 2.2

mpp’s picture

Thanks @hanoii, the patch in #49 applies to 1.2. Some small remarks:

  1. +function draggableviews_views_data() {
    We should add the dochead:
    /**
     * Implements hook_views_data().
     */
    
  2. +  protected function defineOptions() {
    

    This is missing a dochead (can user inheritdoc):

      /**
       * {@inheritdoc}
       */
    
  3. +      $config = \Drupal::config('views.view.' . $view->id());
    

    We should use dependency injection for the config.

  4. +    $form['source'] = array(
    

    Use the [] array notation

mpp’s picture

@hanoii,

Applied the patch from #48.

When I try to edit the relationship I get this Notice: Undefined index: fields in DraggableViewsRelationship.php on line 63.

After doing a drush cr, the following message appears: Plugin ID 'page_1' was not found.

mpp’s picture

Status: Needs review » Needs work

We may need to implement setOptionDefaults()

frederickjh’s picture

This may need some more work. Using version 1.2.0 I applied patch 48 from comment 49. However when I went to add the Draggableviews: Content relationship I got AJAX errors and am unable to edit or configure the relationship after adding. I am attaching three text files with errors from Google Chrome's inspector, in hopes that they will be of use to figure this out.

  1. Error on adding the relationship by clicking on the Apply (all displays) button
  2. Error on clicking on the button to Add and configure relationships (dialog never appears).
  3. After exiting the Add relationships dialog by canceling, Clicking on the name of the relationship in the Relationships list also resulted in an error.

Because of these errors and the dialog that does not appear I cannot mark this relationship as required.

I tested this by adding other relationships with and without the patch. Adding and configuring these all work with out any AJAX errors.

frederickjh’s picture

I switched to the 1.0.0 version and tested patch 44 with the same results as above. For me without the ability to make the relationship required this makes these patches unusable as the displays show doubles. The site is on Drupal 8.6.0 if that makes any difference.

Chris Matthews’s picture

Wondering if there is any progress on this issue as the module is essentially unusable at this point.

pingevt’s picture

FileSize
5.83 KB

Throwing this out there to begin with, I've read through the comments, but I haven't tried all the different patches recently. I know I used one on a project awhile ago (and still had to patch it a little more), but I don't fully remember which one and how I did it.

The discussion about using relationships is interesting, but I disagree with using a relationship. In my opinion the editor should not have to use so many steps to create a sort. They should just be able to create a sort. But I do understand how the relationship creates added flexibility. I think what you want is what you get from how a relationship plugin query is built, rather then the default query for a sort. The way I created this is a new sort will join the table again with a new alias which will give that flexibility and take care of the problem mentioned in comment #35.

Attached is my pass at fixing the overall issue.

  • Everything is in one Sort plugin
  • You have the option to sort from any View/Display that contains a Draggable Content Field Plugin
  • If you just want to select the current view/display, you can choose that, otherwise, you would need to save the view before being able to select it
  • You have the choice to add content that is new (or just not present in the sort) to the beginning or end of the list

I've tested through a few scenarios. A normal one sort, one display. Multiple sorts, one display.

I don't believe this will require any update hooks, as it should fallback gracefully and there are no DB updates (and quite honestly, I believe this module to be totally ineffective without some patch to it to grab the correct sort weight anyways)

The patch was agains the development branch and D8.7.8.

pingevt’s picture

One issue I found with this patch while playing with it today... If you only attach the content to the master display you can't target the individual view displays. Easy work around should be to just always overwrite for each display. I'll work on updating that.

Also, I didn't look into the weight field display plugin to show the correct weight. Didn't even test it yet. This will need to be worked on as well. I imagine could be very similar to the sort plugin though.

pingevt’s picture

FileSize
6.21 KB

Update for master display not overriding

kerasai’s picture

I tested #58 on an clean Drupal install and it worked as expected. Unfortunately it had the Undefined index: fields issue when applied to a project site.

Turns out that not all view displays have the fields set--consider a view that shows rows of rendered content or users instead of "fields". This bit of code analyzes all views on the site to see which ones may be used to "source" ordering data so if you've got any views rendering non-fields type rows you'll encounter the issue.

Attached is #58 with with additional changes necessary to handle this case. This seems to be working as expected.

kerasai’s picture

Status: Needs work » Needs review
FileSize
6.34 KB
786 bytes

Whoopsies, had an error in the conditional that checks that fields is an array.

Fixed up patch attached.

bsnodgrass’s picture

#60 deployed and working well for us, cleared our open issues on this.

Bob Snodgrass

pingevt’s picture

#60 seems to be working well for me too. I'm working on fixing Draggableviews Demo module with these updates as well as fixing and adding to the tests. Hope to have another patch shortly.

pingevt’s picture

I updated Draggableviews Demo module as well as added a few test. I'm somewhat new to tests, so someone may have a better idea about that, but the big thing that came out of that was updating some schema files for the options in the view sort plugin. That was good to learn...

Another to-do here might be to add those same options to the Draggableviews weight Field plugin so you can choose which weight field to display. I don't want to create an issue though until this is committed.

pingevt’s picture

And of course i uploaded the wrong file....

hanoii’s picture

FileSize
8.11 KB

So, this is a reroll of my patch at #49 against the current dev.

I need this as a project relies on this approach.

While I've been monitoring this issue I couldn't get to review it properly. The fact that we now have two complete different approaches on the same issue makes it hard to follow and review. At least we should attempt to reach consensus into one approach. I have am helping maintaining the module and have commit access but I don't want to rush any decision here. What others things regardless of the last few who submitted and commented on the latest patch?

On #33 @iStryker shared some doubts but then there were some compromises.

@pingevt and @kerasai are you sure your approach cover all of the use cases I mentioned and documented above? some of those are on #36. Also there we were exploring the possibility of having both options. A more complex sort handler and the option to use relationships. Would you consider attempting to merge both functionalities?

ahebrank’s picture

Reroll of #64 to support the new join plugin (https://www.drupal.org/project/draggableviews/issues/2903567) in dev. This allows a sort that's both specific to an argument(s) and based on a custom display.

TwiiK’s picture

I just tried this module, but immediately ran into duplicate entries when I tried sorting my views. Using the dev branch with the patch in #66 fixed the duplicate entries. Everything seems to work as expected so far.

alison’s picture

Status: Needs review » Reviewed & tested by the community

This patch worked for me when the module simply did not work at all -- even the "demo" module, the weights worked on the "order source display" but they were non-existent on the display I was actually showing, if you know what I mean (i.e. I added "draggable weight" field to the view output and it was "0" for every result -- after using this patch and configuring the sort handler to point to a specific view display, everything worked the way I expected it to).

The one thing that still doesn't work for me is using two weights in the same view -- i.e. I'm using "group by" with a node reference field, and I had another view display to custom-order the referenced nodes so I could customize the order of the "group by" nodes and the view results under each grouped section, but, for now, I can live with that. (I tried #2867348: Not working with Group By feature before without success -- then I tried the patch on this thread, but I didn't try adding #2867348 after adding the patch on this thread, too worn out from an afternoon of frustrating troubleshooting and need to move on :) Just sharing all the details in case it's useful for someone else!)

Anyway! #66 works great for me, so with #67's feedback and now mine, I'm marking RTBC -- obviously, module maintainer(s) can change it back :)

Thank you everybody!!

rwilson0429’s picture

Patch in #66 works great for me even when filtered by contextual filters and relationships. Thanks @ahebrank

Adrian83’s picture

The patch in #66 would not apply for me without a new line with space at the end.

iStryker’s picture

How does this patch work for exisiting draggableviews views?

rho_’s picture

Patch #66 applied cleanly for me against 8.x-1.2+9-dev, and works nicely. Watchdog appears clean after some use.

loze’s picture

Patch works. Thanks.

KurtTrowbridge’s picture

Is anyone successfully using this patch with version >=8.9.2 of Drupal core? I was using this without issue for the past few months, but now it seems to be broken again (I have a table where the weights of staff members are set via drag-and-drop that doesn't maintain the order after I save it, and it incorrectly matches the order on the staff page to begin with), and I don't see anything else in recent updates to the site that would have done it. Thanks!

loze’s picture

@KurtTrowbridge I'm using it with 8.9.3 without any issues

mistergroove’s picture

Is this functionality in the 2.0 branch?

iStryker’s picture

Currently 8.x-1.x-dev and 2.0.x are the same. Therefore need the patch as of right now. I am waiting for people to fix and confirm the patch for which breaks 2.0.x then can move ahead with other patches/fixes.

devad’s picture

Latest patch #70 is made for 2.0.x-dev.

It failed to apply to 8.x-1.x branch due to this change: #3104293: Remove the 'core' key from Views configuration

So, I have simplified it a bit into patch #78. Stripped out all demo and tests parts and kept only basic code changes.

Now it applies to both 2.0.x-dev and 8.x-1.x-dev.

I have tested it with 2.0.x-dev + D9.0.6 and it works like a charm:

  • Sort view and display view can be two separate views now
  • No problems in sorting views with contextual filters
  • No problems with exposed filters eather.
  • And no problems with duplicates.

I did have to update both my display view and sort view in order for sorting to start to work. I removed draggable content field from sort view and added it again. I also updated sort criteria in both views in order to connect them to work together.

So, some kind of release note should be added to documentation so that the users know that they need to re-connect/refresh all their draggable views sort-display pairs after update.

I tested with sort view and my display being two separate views, as this is the most important new feature of this patch.

Patch attached. You can use it with both 8.x-1.x-dev and 2.0.x-dev. It should apply cleanly to both since these two branches are currently (almost) the same.

devad’s picture

devad’s picture

Patch #70 is for 2.0-x branch, and patch #81 is for 8.x-1.x branch.

They both apply cleanly to respective branches.

Interdiff does not exist actually. It's just a suttle 1 line difference in one file not visible in interdiff.

devad’s picture

[Off topic] Strange. It seems that Drupal.org files list sorting has somehow became broken... sorted by ASC... which is wrong. I hope it is just this issue, not at drupal.org globally. :)

devad’s picture

There is one line difference between two branches at the moment. It's introduced by this patch:

#3104293: Remove the 'core' key from Views configuration

Is this change very important? Can we revert it from 2.x branch... or commit it to 1.x branch as well? Just to make two branches equal for now - to make automated tests and manual testing here easier.

If we are not going to make them equal, then it would be nice if we can turn on automated tests here for 2.0.x branch for proper testing purposes.

devad’s picture

One test in patch #81 didn't pass.

@pingevt can you take a look maybe? Or someone else of course...

I am not very familiar with tests.

Changing to "Needs work".

devad’s picture

Status: Reviewed & tested by the community » Needs work
devad’s picture

wombatbuddy’s picture

The patch #78 does the job (Drupal 8.9.6, Draggableviews 2.0.x-dev).

devad’s picture

Status: Needs work » Reviewed & tested by the community

According to comment #87 about patch #78 lets switch this issue to RTBC.

I would like to suggest that we commit patch #78 to both 1.x and 2.x branch and then continue with demo adjusting and demo-tests adjusting in a separate issue.

To have handler and order in two separate views is a new approach and I believe a future of this module. And both demo and demo-tests require quite some work to adjust to this new approach.

So, I vote not to wait for them now since the main functionality is kinda broken and it is better to fix it asap.

theRuslan’s picture

I can confirm that patch #78 works properly.

devad’s picture

I couldn't manage for patch #70 + D9 to work properly with my view which has contextual filter.

I needed all my content type views (both with contextual filters and without contextual filers) to have nodes sorted in the same order... so I have switched to Weight module and it works.

If you need different sorting for various views than you will need multiple weight fields added to your content type / entity.

loze’s picture

Confirming that #78 applies cleanly and works. +1

gthing’s picture

Patches from #70, #81 do not apply for me in Drupal 9.x w/ 2.0.x-dev

git apply -v draggableviews-sort_handler_specify_order_view-2767437-78.patch
Checking patch config/schema/draggableviews.views.schema.yml...
Checking patch draggableviews.module...
error: while searching for:
    \Drupal::messenger()->addMessage(t('There was an error while saving the data. Please, try again.'), 'warning');
  }
}

/**
 * Implements hook_views_query_alter().
 */
function draggableviews_views_query_alter(ViewExecutable $view, QueryPluginBase $query) {
  // Add additional `join condition` for fixing table join.
  // Cannot add it in the draggableviews_views_data_alter because we need view
  // data.
  if ($query instanceof Sql && isset($query->getTableQueue()['draggableviews_structure'])) {
    /** @var \Drupal\views\Plugin\views\join\Standard $join */
    $join = $query->getTableQueue()['draggableviews_structure']['join'];
    $join->extra = [
      [
        'field' => 'view_name',
        'operator' => '=',
        'value' => $view->id(),
        'numeric' => FALSE,
      ],
      [
        'field' => 'view_display',
        'operator' => '=',
        'value' => $view->current_display,
        'numeric' => FALSE,
      ],
    ];
  }
}

error: patch failed: draggableviews.module:219
error: draggableviews.module: patch does not apply
Checking patch src/Plugin/views/sort/DraggableViewsSort.php...
igonzalez’s picture

#78 works well in 2.x branch

gthing’s picture

I got this working with patch #78.

However, I cannot get the draggableviews weight to work on views that use the Serializer format. It shows up as 0 despite defining the relationship to the sorting view in the draggableviews sort criteria.

edit: this actually does work, but it appears it break when MULTIPLE contextual filters are used.

SaraT’s picture

I applied #78 in 2.0 on D8.9.13. My duplicates are gone, and my sort order is saving and matching among the sort view and the display view. I'm not getting the sorting results I'm trying to replicate from a D7 site, though.

I have a content type of "Resource" where each node can have multiple "Topic" taxonomies assigned. When I use the Topic A filter on my sort view, it saves Nodes 1, 2 and 3 in the order I wish, but when I encounter the same nodes (plus some additional) when filtering by Topic B, I cannot resort the same nodes without it affecting the sort order for Topic A. For example, I'd like the order Node 1 then 2 for Topic A, While I'd like Node 2 to follow Nodes 3 and 1 in Topic B.

I'm not a programmer/developer, so I have no idea if the module is intended to work this way. If it is intended, it's not working for me.

Adrian83’s picture

@SaraT At one point, this Args Not Working issue had given us this feature, committed December 26, 2019. However, seeing some of the recent issues in the queue, I am not able to verify that the latest release is still working in this way.

zebda’s picture

I'm also experiencing trouble with Draggable Views and Contextual Filters. The patches found in the Args Not Working issue don't work for me. I'm running Drupal 9.1.5 and Draggable View 2.0. Any suggestions on how to solve this?

devad’s picture

@zebda #97. You can try solution from comment #90. It did work for me.

weekbeforenext’s picture

weekbeforenext’s picture

Just exposing the #78 patch since a lot of folks are using it and it passes tests.

zebda’s picture

@devad #98, I bumped in to the Weight module earlier but the description of the project page is not very informative. But this is a perfect solution. Thanks!

joco_sp’s picture

I just applied #78. It applied with no problem and so far it seams that it is working. It worries me a little, because people have problem with Contextual filters. Will try it out and post my findings.

PS: a part of the solution, that is presented also in this patch, you can find also in this patch. I didn't try it, because I really like the possibility to select from which View you want to use the draggable order. Maybe if somebody have issues with Contextual filter, can try the mentioned patch. Maybe it will work.

nimoatwoodway’s picture

#78 Does the trick for me. So far no negative side effects noted. Thanks!

spacerabbit’s picture

Just trying the #78 patch and it works as a charm so far.
Just wanted to say 1000 Thanks to @iStryker for this amazing module (that solves many of my problems) and of course, thanks to all the contributors, @devad you made my day.

fpoirier’s picture

Can confirm that duplicate issue is back with #78 and D9.1.6. Thanks !

ssantaeugenia’s picture

Drupal version 9.1.6
after applying patch # 78 or # 70 I get the following error in the view

--------------------------------
SQLSTATE[42883]: Undefined function: 7 ERROR: function isnull(integer) does not exist LINE 1: ...d_data"."langcode" AS "node_field_data_langcode", ISNULL(dra... ^ HINT: No function matches the given name and argument types. You might need to add explicit type casts.: SELECT "draggableviews_structure"."weight" AS "draggableviews_structure_weight_1", "node_field_data"."created" AS "node_field_data_created", "node_field_data"."nid" AS "nid", "node_field_data"."langcode" AS "node_field_data_langcode", ISNULL(draggableviews_structure.weight) AS "draggableviews_structure_weight" FROM {node_field_data} "node_field_data" LEFT JOIN {draggableviews_structure} "draggableviews_structure" ON node_field_data.nid = draggableviews_structure.entity_id AND (draggableviews_structure.view_name = :views_join_condition_0 AND draggableviews_structure.view_display = :views_join_condition_1 AND draggableviews_structure.args = :views_join_condition_2) WHERE ("node_field_data"."status" = :db_condition_placeholder_3) AND ("node_field_data"."type" IN (:db_condition_placeholder_4)) AND ("node_field_data"."langcode" IN (:db_condition_placeholder_5)) ORDER BY "draggableviews_structure_weight" ASC NULLS FIRST, "draggableviews_structure_weight_1" ASC NULLS FIRST, "node_field_data_created" DESC NULLS LAST; Array ( [:db_condition_placeholder_3] => 1 [:db_condition_placeholder_4] => empleat [:db_condition_placeholder_5] => ca [:views_join_condition_0] => empleat_ordenacio [:views_join_condition_1] => page_1 [:views_join_condition_2] => [] )

------------------------------

Any solution ?

Thanks

Marios Anagnostopoulos’s picture

Works for me for core version 9.1.5

firfin’s picture

@106 Can you reproduce this error with a clean install? And are you sure you are using version 2?

Patch from #78 Works great for me on 8.9.14 and DV2.0.0
You do need to alter the existing 'listing' views sort handler to set the correct sorting view and display.
Also solves the problem in #3153830: DraggableViews not working after update to 2.0.0, which I think is actually a duplicate of this issue as the solution here makes the problem there non-existent.

gooddev’s picture

Patch #78 works! Thank you!

ashu1629’s picture

Patch #78 works! Thank you!

jastraat’s picture

Unfortunately this patch did not address the problem in our case. I believe the issue is that the draggable view ordering does not work if the view is taking an argument. And this patch didn't address that.

wmcmillian-coalmarch’s picture

wmcmillian-coalmarch’s picture

I've updated the patch in #78 to include an option for passing the contextual filters to the display.

This is my attempt at solving the issue where contextual filters are causing the view not to sort.

iStryker’s picture

Status: Reviewed & tested by the community » Fixed

Status: Fixed » Closed (fixed)

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