If a view uses ajax, and user can able to change Items per page value + pager exits, the pager won't work as well, if 'items per page' has different value, than the original view's pager option.
To reproduce: Create a view pane, Set pager to 'Paged output, full pager', and set 'Items to display' on views pager settings form to 10. On pane configuration let the 'Allow settings' check 'Items per page'.
Then add a new pane, and set 'Item per page' on pane input form to 1.

On page load, the 1 item will show up with pager. Then use pager, the result will be 10 item, as in the original view was set up.

This is, because the ajax callback of views just load the default view settings, and give back $this->preview() as result. No chance to add the pane's own parameters.

I'm not sure how do we need to handle this.. I tried to extend the preview() in views_content_plugin_display_panel_pane.inc, but I didn't get any relevant information from exisiting variables. The one of my idea to pass number of items by url, but I'm not sure, this is the best way (I think, we could have other pane settings, which would be lost)

CommentFileSizeAuthor
#84 interdiff.txt846 bytesdsnopek
#84 views_content-keyword-substitution-1910608-84.patch12.05 KBdsnopek
#83 views_content-keyword-substitution-1910608-83.patch12.02 KBfrodri
#79 1910608-ctools-ajax-settings-lost-79.patch17.39 KBJorrit
#76 1910608-ctools-ajax-settings-lost-76.patch17.57 KBJorrit
#74 1910608-ctools-ajax-settings-lost-74.patch16.87 KBJorrit
#73 1910608-ctools-ajax-settings-lost-73.patch16.81 KBJorrit
#56 views_content-keyword-substitution-1910608-56.patch12.07 KBmpotter
#51 views_content-ajax-1910608-51.patch767 bytesiSoLate
#48 ctools_cache_table_fix_1910608_48.patch665 bytesrklawson
#36 views_content-keyword-substitution-1910608-36.patch11.97 KBlucas.constantino
#33 views_content-keyword-substitution-1910608-33.patch11.94 KBeyilmaz
#29 views_content-ajax-1910608-29.patch11.71 KBmkhamash
#28 views_content-ajax-1910608-28.patch11.69 KBredndahead
#27 views_content-ajax-1910608-27.patch11.66 KBgmercer
#26 views_content-ajax-1910608-24.patch11.61 KBgmercer
#24 views_content-ajax-1910608-23.patch12.11 KBmkhamash
#23 views_content-ajax-1910608-23.patch12.28 KBmkhamash
#16 views_content-ajax-1910608-16.patch11.42 KBvordude
#13 1910608-views_content-ajax-13.patch11.41 KBmpotter
#12 1910608-views_content-ajax-12.patch12.48 KBmpotter
#11 1910608-views_content-ajax-11.patch12.48 KBmpotter
#7 1910608-views_content-ajax-7.patch12.87 KBbeeradb
#6 1910608-views_content-ajax-6.patch12.86 KBbeeradb
#5 1910608-views_content-ajax-5.patch21.05 KBbeeradb
#4 1910608-views_content-ajax-4.patch12.72 KBhefox
Support from Acquia helps fund testing for Drupal Acquia logo

Comments

merlinofchaos’s picture

This is a known, unfixable issue, and there is warning text in the pane settings that addresses this.

Well, okay, I just went and looked; that warning text actually doesn't appear due to D6 -> D7 upgrade changes that got missed. Go figure. I'll have to fix that.

vflirt’s picture

Hi,

all i can say is that none of the allowed settings (pager element, offset, items per page, etc.) actually work with ajax. I see that in views module : includes/ajax.inc that is a line :
$view->display[$display_id]->handler->set_option('pager_element', $pager_element);
that should set he pager element but it has 0 influence on the result.
The easiest step to reproduce :
create test page , create test view with content , add content page display , set it to ajax , allow page settings to be set in the panel and set the pager element to be 0 and items per page to be 2 (for the test) , attach the view pane in the panel and set page element to be 1 and items per page to be 1. Add 5 nodes so we have some content.
Now when you open the page /test (or whatever is the url) that we will see 1 node with pager as expected and the linkst are correct page=0,1 or page=0,2 etc.
But when you click any of the pager it just reloads 2 nodes and loads the first page not the one you selected and when you check the pager links they are page=1,1 or page = 2,1 etc. So the pager handler has 0 clue of the settings that were posted trough ajax.

I will try to dig more into it this days and hope to find solution as I need to for one of my projects but I can't promise how far I will go and if the client won't just accept to reload the page as then the listings are working correctly.

Kind Regards

vflirt’s picture

Hi,

a few more lines i can add to this but all are connected with view module.
Instead of :

$view->display[$display_id]->handler->set_option('pager_element', $pager_element);

you should use the pager array options :

$options = $view->display[$display_id]->handler->get_option('pager');

and then setup the ones posted from ajax :

$options['options']['id'] = $pager_element;
$options['options']['items_per_page'] = $items_per_page;
$options = $view->display[$display_id]->handler->set_option('pager', $options);

then we will have to add :

$items_per_page = isset($_REQUEST['items_per_page']) ? intval($_REQUEST['items_per_page']) : NULL;

a bit above so we actually have this setting.

and in theme/theme.inc in the template_preprocess_views_view function in the ajax settings we will have to add :

'items_per_page' => isset($view->query->pager) ? $view->query->pager->get_items_per_page() : 0,

as the pager_element is added.

Probably adding a function to the plugin display base class might look cleaner approach in order to handle this situation but that should also require to use array for pager settings in the ajax so other modules could add settings that would need but i guess that would require too much change in the views module.

I think that this could be related more to the views module to provide either option so other modules could add their own javascript settings and also to process them but again that is too much to ask. I am not that good to write such patch for the views module to provide such extendibility but the above changes does work quite well for me so i will commit a patch soon.

Kind Regards

hefox’s picture

Title: Ajax + Allow settings: Items per page: pager won't work » Ajax + Allow settings: Allowed settings lost on ajax (exposed forms/pager)
Status: Active » Needs work
FileSize
12.72 KB

This is more a proof of concept but can defiantly be rolled into a true patch if think it's worth it.

Patch applies after #1901106: Allow querystring to (optionally) override empty exposed filters. as too lazy to make two patches for a POC type patch

So what patch does

1) Moves the 'changing of the view' into the display handler in a method pane_process_conf
2) in set_pane_conf, set the pane config both to the object and to the cache (via cache_set( with a CID that includes the dom id to cache table, @todo move that to own cache table, perhaps one done similar to cache_form that uses expires to last through cron)
3) Add a hook_views_pre_view that triggers pane_process_conf
4) in pane_process_conf if no pane config, look for it in cache and use/set that if exists.
5) Adds a views_content_views_pre_view that triggers the function, and uses module implements to execute it first so anyone else currently doing views_pre_view can use the config (had stuff break when trying to do it in ..pre_execute?)

Use of cache is because
1) didn't want to try and figure out where that view was coming from and loading that
2) didn't want to pass around the settings in a way a user could manipulate

beeradb’s picture

The previous patch failed to work with clickable table sorting. Pane configuration would trump everything else. This updated patch checks for clicksorting before applying the pane sort configuration.

beeradb’s picture

Oops, looks like the previous version included the first patch. Here's a retry.

beeradb’s picture

It looks like this patch was also based on #2016559: Non-numeric pager_id causes a loop that just won't stop, here's a re-roll based on that as well.

So to get this patch working:

Apply #1901106: Allow querystring to (optionally) override empty exposed filters.
Apply #2016559: Non-numeric pager_id causes a loop that just won't stop
Apply this patch.

SocialNicheGuru’s picture

patch does not apply cleanly
I did follow the instructions and applied the two others before the one attached in 7

july 5 ctools

patch -p1 < 1910608-views_content-ajax-7.patch
patching file views_content/plugins/content_types/views_panes.inc
Hunk #2 FAILED at 163.
1 out of 2 hunks FAILED -- saving rejects to file views_content/plugins/content_types/views_panes.inc.rej
patching file views_content/plugins/views/views_content_plugin_display_panel_pane.inc
Hunk #2 succeeded at 472 (offset 1 line).
patching file views_content/views_content.module
Hunk #1 succeeded at 333 (offset 13 lines).

boyan.borisov’s picture

Hi guys,

I had the same problem but I found a view's related solution. See the link bellow

https://drupal.org/node/2065975#comment-7757969

vflirt’s picture

Hi,
as pointed the patch doesn't apply clean.
I applied it manually and it is working great and fixes number of other issues with views in panels and ajax.

There is one minor issue though. If you set the content pane to allow path overwrite and then enter path in panel config it produces call to unidentified function ctools_context_keyword_substitute on line 544 of ctools/views_content/plugins/views/views_content_plugin_display_pane.inc

the code that does it is :

if (!empty($conf['path'])) {
      $conf['path'] = ctools_context_keyword_substitute($conf['path'], array(), $contexts);
    }
    if ($allow['path_override'] && !empty($conf['path'])) {
      $this->view->override_path = $conf['path'];
    }
    else {
      if ($path = $this->get_option('inherit_panels_path')) {
        $this->view->override_path = $_GET['q'];
      }
    }

all i did was to add ctools_include('context'); before that call of that function and it was working like a charm.

I think what this patch does is great and should be used.

Kind Regards,
Dobromir

mpotter’s picture

Issue summary: View changes
Status: Needs work » Needs review
FileSize
12.48 KB

OK, let's take a cleaner approach here. This patch is too intertwined with #1901106: Allow querystring to (optionally) override empty exposed filters.. That patch is adding additional functionality. The patch in this issue really addresses the fundamental issue of ajax losing options from the pane config. So it's really this issue that should be committed first (and soon) and then I will go re-roll 1901106 so it applies *after* this patch.

Also, while #9 addresses the specific pager settings in views, it doesn't address the more fundamental problem here in ctools related to other config options being lost by Ajax.

Here is a re-roll of this patch for the current 1.4 version of ctools. It has no other patch dependencies.

While the diff is ugly, what this patch does is relatively simple. It is taking code *out* of the views_content_views_panes_content_type_render() function in views_content/plugins/content_types/views_panes.inc and moving it *into* a new pane_process_conf() function in the handler views_content/plugins/views/views_content_plugin_display_panel_pane.inc.

By moving this code into the new pane_process_conf() function of the handler, it can then be called from both the original views_content_views_panes_content_type_render() function as well as a new views_content_views_pre_view() hook which preserves all of the pane config when using ajax.

Hopefully some others can review this. Don't be scared away by the size of the diff. I believe this puts the common code into the proper location and handles the ajax more generally for all config options.

We really need to get this patch committed asap so other functionality patches like 1901106 can be reviewed. Because any other patch that tries to add pane config options conflicts with this patch since this patch moves the code to a different file.

mpotter’s picture

Ack, patch in #11 was missing a line. Here is the fixed patch.

mpotter’s picture

OK, 3rd time is a charm! The patch in #12 contained some contamination from the #1901106: Allow querystring to (optionally) override empty exposed filters. issue. This patch should be clean. Crossing fingers.

mansspams’s picture

pokurek’s picture

Hi,
there is a small bug in #13 around line 557.
$view->set_exposed_input() has to be $this->view->set_exposed_input().
Regards,
Petr

vordude’s picture

This works for me.

My tests were specifically focusing on using panel pane configuration to override 'items per page' returned in a view pane with an ajax pager.

#13 still applies cleanly, I've rerolled with the addition from #15.

ParisLiakos’s picture

Status: Needs review » Reviewed & tested by the community

#16 works great for me..i tested it with both offset and items per page

The last submitted patch, 4: 1910608-views_content-ajax-4.patch, failed testing.

The last submitted patch, 6: 1910608-views_content-ajax-6.patch, failed testing.

The last submitted patch, 5: 1910608-views_content-ajax-5.patch, failed testing.

The last submitted patch, 7: 1910608-views_content-ajax-7.patch, failed testing.

ParisLiakos’s picture

Status: Reviewed & tested by the community » Needs work

#10 is still an issue though

mkhamash’s picture

Re roll for patch #16 against ctools-1.6 the current dev. this will need another review since views_content_views_panes_content_type_render have been edited since the last time this patch have been updated.

mkhamash’s picture

FileSize
12.11 KB
redndahead’s picture

Status: Needs work » Needs review
gmercer’s picture

We found that the previous patch #24 wasn't working when we pass view context filter/arguments to a content pane on a page.

In the new patch we use the pane_contexts passed in to get the view filter/arguments for the pane.

gmercer’s picture

Ah... we found a small issue in my previous patch. We weren't getting the panel args correctly in all cases. Using $panel_args turns out to be a better (yeah) approach. So here's another try.

redndahead’s picture

Forgot to get the panel args variable from the conf variable.

mkhamash’s picture

FileSize
11.71 KB

Re-roll Patch #28 against 7.x-1.7 to apply, also added the the edit introduced in issue [2416589] to views_content_views_panes_content_type_render() to the new views_content_plugin_display_panel_pane extends views_plugin_display() function.

The last submitted patch, 28: views_content-ajax-1910608-28.patch, failed testing.

hefox’s picture

Haha, I'm glad I saw this when I did and not a minute too soon, cause was about to reroll it, and bam, the patch was posted less then a minute before! Thanks, applies clean (haven't tested past that)

eyilmaz’s picture

With the latest patch, the keyword substitution doesn't work.

Removed code from views_content_views_panes_content_type_render() :

-  if ($allow['exposed_form'] && !empty($conf['exposed'])) {
-    foreach ($conf['exposed'] as $filter_name => $filter_value) {
-      if (!is_array($filter_value)) {
-        $conf['exposed'][$filter_name] = ctools_context_keyword_substitute($filter_value, array(), $contexts);
-      }
-    }
-    $view->set_exposed_input($conf['exposed']);
-  }
-

added instead:

+    if ($allow['exposed_form'] && !empty($conf['exposed'])) {
+      $this->view->set_exposed_input($conf['exposed']);
+    }

should be:

+    if ($allow['exposed_form'] && !empty($conf['exposed'])) {
+    foreach ($conf['exposed'] as $filter_name => $filter_value) {
+     if (!is_array($filter_value)) {
+        $conf['exposed'][$filter_name] = ctools_context_keyword_substitute($filter_value, array(), $contexts);
+      }
+    }
+      $this->view->set_exposed_input($conf['exposed']);
+    }

Refreshed patch as attachment.

joelstein’s picture

Status: Needs review » Reviewed & tested by the community

The patch at #33 worked for me. Thanks!

mdeltito’s picture

Came here to report the same issue as described in #33. Thanks for the updated patch @eyilmaz

lucas.constantino’s picture

Hello,

I've had an issue with the ctools_context_keyword_substitute when the pane configs get cached. The problem is the function is, sometimes, not defined in the time of calling. The solution would be to simply invoke "ctools_include('context')" before the function usage. Anyway, I did not understand if this include was not in code by purpose or if it was simply forgotten. Most times your page would have already loaded CTools contexts anyway, so on most situations the issue would not occur.

Here goes patch #33 with the addition of ctools_include.

IRuslan’s picture

#36 works for me.
+1 RTBC

mkhamash’s picture

Also RTBC #36 working perfectly fine for some time now.

rivimey’s picture

Issue tags: +Dublin2016

Patch applies cleanly to current 7.x-1.x

PHP code snifer is complaining about the class views_content_plugin_display_panel_pane because access modifiers are not defined for functions or (some) variables. However, this appears to be the normal thing for such plugins.

I have set up the scenario indicated (requires panels, page manager, devel-generate) and the patched code works correctly, in that the pager is now using 1 item per page.

Good to go, I think.

rivimey’s picture

  • japerry committed f38dfe8 on 7.x-1.x
    Issue #1910608 by mkhamash, mpotter, beeradb, gmercer, hefox, redndahead...
japerry’s picture

Status: Reviewed & tested by the community » Fixed

Usually something this big wouldn't be committed, but after reading #11, the code, and talking to rivimey about testing, I feel somewhat okay about committing this. Fixed!

Status: Fixed » Closed (fixed)

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

bobodrone’s picture

After this change I experience a huge "cache" table (>5gb) in just the last 10 days. Since yesterday when I clear the cache I have a "cache" table of:

19647 rows in total
19600 of those are beginning with: "view_panel_pane_"[HASH]
Table size in the last 12 hours: 1GB (and growing every minute)

In the commited patch you can find:

-  function set_pane_conf($conf = array()) {
+  function set_pane_conf($conf = array(), $set_cache = TRUE) {
     $this->set_option('pane_conf', $conf);
+    $this->view->dom_id = !empty($this->view->dom_id) ? $this->view->dom_id : md5($this->view->name . REQUEST_TIME . rand());
+    if ($set_cache) {
+      cache_set('view_panel_pane_' . $this->view->dom_id, $conf);
+    }

I have no clue myself what´s going on there.
Anyone knows what the problem can be?

// Fredric

rivimey’s picture

Hi @bobodrone, thanks for your report.

I cannot explain the issues you are having, but the lines you highlighted are creating and caching the saved configuration for a pane (I think mostly for use by Views AJAX calls). The cache_set call accepts up to 4 args; the first two (present here) are the key name and the data to save. The last two are the cache name and the timeout, which defaults to permanent.

On looking at this code I am somewhat confused as to why the caching is happening at all, but it would seem reasonable to set the timeout to some form of 'temporary', else the cached entries will not be removed when a general clear is done.

This change could at least assist with cache cleaning:

cache_set('view_panel_pane_' . $this->view->dom_id, $conf);

to

cache_set('view_panel_pane_' . $this->view->dom_id, $conf, 'cache', time() + 3600);

Which means it can (not will) now be cleaned up by a general cache-clear after an hour's life.

Overall I think some more investigation is needed as to why there are so many cache_set operations being performed in the first place. Perhaps some judicious logging code in the set_pane_conf() function would do that?

KarlShea’s picture

This patch breaks any changes to views arguments set when making an AJAX call.

KarlShea’s picture

If I change line 492 to

if (empty($this->view->args)) {
  $this->view->set_arguments($args);
}

My AJAX call stops failing. Does that break the intentions of this patch?

rklawson’s picture

I've also experienced the cache table growing excessively large as a result of this patch. Since the patch here has already been released, I rolled a light-weight patch to change the default CACHE_PERMANENT of cache_set() to CACHE_TEMPORARY. This will probably end up needing a separate issue.

I agree with @rivimey and also do not understand why caching has been implemented in this way and think that the implementation here should be re-examined because of its negative impact on performance (and disk space).

geek-merlin’s picture

jastraat’s picture

iSoLate’s picture

It seems it is setting the cache one sided not taking into account the cache settings on a views pane. In my case, this is breaking my glossary pager.
Quickly added a small patch.

japerry’s picture

Status: Closed (fixed) » Needs work

Re-opening because I had to revert it.

mpotter’s picture

Commit was reverted on 2017-01-19. Adding the 1.13 release as a parent. This needs to get resolved since Open Atrium sites are using this functionality and removing it from the next release will break sites.

Alan D.’s picture

Pre-revert, it was potentially killing sites on smaller hosting plans, it's appeared to have sucked up 350 MB within a week for one of our clients.

I would assume a direct revert, but I didn't check. So to get the committed patch for the revert:

http://cgit.drupalcode.org/ctools/commit/?id=b4d7d11

DamienMcKenna’s picture

This didn't get into 1.13, maybe it will get into 1.14.

We need to try getting this to RTBC.

mpotter’s picture

Here is a reroll of #36 that applies to 1.14. Doesn't address any of the caching issues that were raised, so those still need work.

Need to decide if the small fix from #51 should also be added.

mpotter’s picture

Status: Needs work » Needs review

OK, after reviewing all the related issues, going to leave #56 as is and we should address the cache fix in #2843333: Fix the Ajax settings fix's broken cache usage. Then I'll go update the related #1901106: Allow querystring to (optionally) override empty exposed filters..

This is getting awfully complicated, but we really need to fix these Ajax issues as there are thousands of Atrium sites that need this (without breaking thousands of more sites with large cache issues).

Anybody else out there have some time to help get all these issues to RTBC?

The last submitted patch, 51: views_content-ajax-1910608-51.patch, failed testing. View results

joelstein’s picture

Status: Needs review » Reviewed & tested by the community

Thank you for the patch in #56! It applies cleanly and fixes the missing pane settings on ajax. Also, the caching issue seems to be resolved with the patch in #2843333-9: Fix the Ajax settings fix's broken cache usage.

joseph.olstad’s picture

+1 yes we are depending on this patch too.

we have an exposed form in our view on a panel page, with an apply and reset button
the reset button without this patch redirects to the home page rather than resetting the form

with the patch, the reset button works correctly.

Jorrit’s picture

Patch #56 is causing problems for me, which appeared after I updated to OpenAtrium 2.6.29. The symptom is that the preview AJAX callback on the the views edit page gives the following error:

Fatal error: Nesting level too deep - recursive dependency? in ....../drupal/profiles/openatrium/modules/contrib/ctools/views_content/plugins/views/views_content_plugin_display_panel_pane.inc

This is caused by the following line of code:

if ($set_cache || $conf != $this->get_cached_conf()) {

It happens because != compares objects with recursive properties. The only recursive objects are in the 'pane_contexts' key of the $conf variable. These objects are quite large and I think it would be beneficial also for performance when those large objects would not be serialized at all in the Drupal cache.

For me, this can be reproduced on OpenAtrium on the `admin/structure/views/view/oa_fullcalendar/edit/oa_calendar_month` page when it is visited after (in the same session, by the same user) a calendar with the output of this view is displayed.

surendramohan’s picture

I 100% agree with you Jorrit, and as far as Open Atrium is concerned, it's not limited to an edit page of any view.
We should be able to even reproduce it on each and every page that uses AJAX; especially the "Load more" button (which I tested and am able to reproduce each time I try to use this feature; regardless of the page).

And as correctly said, this issue popped up after the latest OA upgrade, that is, openatrium-7.x-2.629) .

surendramohan’s picture

I applied patch #56 and it works great at my end. Thanks Mr. Potter for this patch.

In case of Open Atrium 7.x-2.629, just the below mentioned part of the patch does the job; other code lines from the patch are already applied to the module:

File name:
/views_content/plugins/views/views_content_plugin_display_panel_pane.inc

+    $this->view->dom_id = !empty($this->view->dom_id) ? $this->view->dom_id : md5($this->view->name . REQUEST_TIME . rand());
+    if ($set_cache) {
+      cache_set('view_panel_pane_' . $this->view->dom_id, $conf);
+    }
joseph.olstad’s picture

I just triggered a php 7.2 test on patch 56

7.2 is now supported by Drupal 7.x core (dev branch) , the next tagged release of 7.x core (7.60) will support php 7.2 so we are best to start testing contrib now. if there's any 7.2 issues we should open a new issue and put a reference to it here.

joseph.olstad’s picture

php 7.2 passes

DamienMcKenna’s picture

debain’s picture

For openatrium-7.x-2.631 (fresh install > ajax error) the function should look like this:

  public function set_pane_conf($conf = array(), $set_cache = TRUE) {
    $this->set_option('pane_conf', $conf);
    if ($set_cache){
      cache_set($this->get_conf_cid(), $conf, 'cache_panel_conf');
    }
    $this->has_pane_conf = TRUE;
  }
bobbygryzynger’s picture

Like @debain, in OpenAtrium 7.x-2.631 I am finding that the patch from #56 doesn't seem to have been completely applied to the distribution. The if statement from set_pane_conf looks like this in the distributed code:

if ($set_cache || $conf != $this->get_cached_conf()) {...}

Edit: it appears this patch is getting overridden by another applied to views_content_plugin_display_panel_pane.incin OpenAtrium. See #3000618: Patches to CTools reverse one another.

Jorrit’s picture

I don't think the entire $conf needs to be changed, but just (some of) the results of pane_process_conf(). For instance, a lot of the contents of $conf is needed to determine the $args of the view. I think it is much easier to just cache these $args and not the $conf that was used to create them.

Jorrit’s picture

In #3000618: Patches to CTools reverse one another I have posted a comprehensive patch that fixes this issue while keeping the cache size small and not causing any errors. The patch also includes #1000146: Edit text of "More Link" from Panel config, #1858922: views_content_views_panes_content_type_render() should not set $view->get_total_rows and #1901106: Allow querystring to (optionally) override empty exposed filters.. I am hoping to get some feedback, if anyone is still interested in this problem.

joelpittet’s picture

Jorrit’s picture

I want to separate the AJAX settings fix from #3000618 so it can be considered to be merged on its own in a future ctools release.

Jorrit’s picture

I propose the attached patch. It reduces the size of the cached configuration by first calculating the changes to the views settings using the panel config and contexts, caching that, and reading from that cache on AJAX callbacks. In this way the context objects don't need to be serialized/deserialized.

andyuq’s picture

That was quick Jorrit. It works great now, thanks a lot for your contribution.

ChaseOnTheWeb’s picture

Status: Reviewed & tested by the community » Needs work
Parent issue: #3031732: Plan for CTools 7.x-1.16 release » #3178513: Plan for CTools 7.x-1.17 release

Needs reroll against current -dev

joelpittet’s picture

Jorrit’s picture

Status: Needs work » Needs review
FileSize
17.39 KB

Rerolled

joelpittet’s picture

This patch is quite tricky to review, includes schema changes, lots of deletes, it's hard to know if it will cause regressions... any way this could get a couple tests and/or simplify the fix?

Also there are some minor nitpicks around coding standards (at a glance extra whitespace on the end of lines)

Jorrit’s picture

To be honest, I don't expect that this patch will ever be committed. If it were important to anyone, many more people would have found this issue and commented on it. It is a very complicated change in complicated code. The code worked fine with the project I needed it for, but I am no longer involved in it and I can't contribute time to add tests.

joelpittet’s picture

@Jorrit thanks for the update, I'll take this off my hitlist and wait for the other followers of this issue to chime in.

frodri’s picture

Patch #79 breaks our view pre-render hooks, so I'm rerolling #56 to account for recent changes on ctools.

dsnopek’s picture

Here's a re-roll of patch #83, but where the cache entries expire after 1 hour, in order to work around the cache ballooning issue.