Trying to override pages from archive module I got error
Fatal error: require_once() [function.require]: Failed opening required '/var/www/core7/1/1' (include_path='.:/usr/local/zend/share/ZendFramework/library:/usr/local/zend/share/pear') in /var/www/core7/sites/d7tv/modules/pm_existing_pages/plugins/tasks/pm_existing_pages.inc on line 110
and debuged $arg

array (
  0 => 
  array (
    'ti' => 'archive',
    'pc' => 'archive_page',
    'f' => 'archive.pages.inc',
    'fp' => '',
    'm' => 'archive',
  ),
  1 => 'all',
  2 => '2012',
  3 => '2',
  4 => '13',
)

so usage of array_pop is wrong - needs array_shift() to shift off a first argument
same for content plugin
Fatal error: require_once() [function.require]: Failed opening required '/var/www/core7/1/1' (include_path='.:/usr/local/zend/share/ZendFramework/library:/usr/local/zend/share/pear') in /var/www/core7/sites/d7tv/modules/pm_existing_pages/plugins/content_types/pm_existing_pages.inc on line 51

CommentFileSizeAuthor
#18 1440068-17.patch4.05 KBandypost
#15 1440068-15.patch2.81 KBswentel
#5 1440068-pm_args2.patch2.35 KBandypost
#5 1440068-interdiff.txt834 bytesandypost
#4 1440068-pm_args.patch2.34 KBandypost
#1 1440068-array_shift.patch1.01 KBandypost

Comments

andypost’s picture

andypost
he/him
Primary language Russian
commented
Status: Active » Needs review
StatusFileSize
new 1440068-array_shift.patch1.01 KB

This fixes both places

andypost’s picture

andypost
he/him
Primary language Russian
commented

ctools from git passes arguments same way as rc-1

andypost’s picture

andypost
he/him
Primary language Russian
commented
Status: Needs review » Needs work

By overriding user/register arguments are different

with additional arguments passed (arg1 and arg2) user/register/arg1/arg2

array (
  0 => 'user_register_form',
  1 => 
  array (
    'ti' => 'user_register',
    'pc' => 'drupal_get_form',
    'f' => '',
    'fp' => '',
    'm' => 'user',
  ),
  2 => 'arg1',
  3 => 'arg2',
)

Suppose we need to store original "page arguments" in pm_args too

An open question is hook_update to this model

andypost’s picture

andypost
he/him
Primary language Russian
commented
Status: Needs work » Needs review
StatusFileSize
new 1440068-pm_args.patch2.34 KB

I've added default menu 'page arguments' to special key and fixed both places to use them

andypost’s picture

andypost
he/him
Primary language Russian
commented
StatusFileSize
new 1440068-interdiff.txt834 bytes
new 1440068-pm_args2.patch2.35 KB

Fixed comment wording

swentel’s picture

swentel commented

Looks good on a first glance, but I'll do some more testing this weekend before committing!

andypost’s picture

andypost
he/him
Primary language Russian
commented

@swentel this really broken, please drop some thoughts.

swentel’s picture

swentel commented
Status: Needs review » Fixed

Committed and pushed, thanks!

Status: Fixed » Closed (fixed)

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

swentel’s picture

swentel commented
Status: Closed (fixed) » Active

This broke existing pages that previously worked, I need to revert this patch, do a new release first (it's a critical one in an implementation we have now) and recheck the code.

andypost’s picture

andypost
he/him
Primary language Russian
commented
Status: Active » Needs review

@swentel please point a module that got broken and it's path that does not works with patch?
Probably we need some upgrade function if PM stores config with old values

swentel’s picture

swentel commented

We had two examples with a project here at work:
- the authenticated user pages (although that works fine with the task that page manager itself comes with)
- apache solr search pages with a keyword: eg 'path/searchword'

I'm going to investigate this more tonight.

andypost’s picture

andypost
he/him
Primary language Russian
commented

I got troubles with Archive module because of it's usage of arguments
@swentel do you use apache solr itself or search_api_pages?

swentel’s picture

swentel commented

Apache Solr itself. I'll experiment with the archive module as well.

swentel’s picture

swentel commented
StatusFileSize
new 1440068-15.patch2.81 KB

Initial patch, works fine now for archive and for overriding user pages. Going to test this now with apache solr search pages.

The problem is that we can't be sure that the arguments we need for pmep are at the first or last position of the menu arguments.

swentel’s picture

swentel commented

Ok works fine with apache solr pages now as well, I think this is good to go, but I'll wait until you have reviewed as well.

andypost’s picture

andypost
he/him
Primary language Russian
commented
Status: Needs review » Needs work

I will debug this later today

@@ -267,3 +272,31 @@ function _pm_existing_pages_get_context_arguments($menu_args, $pm_args, $task_id
+  foreach ($args as $key => $value) {
+    if (is_array($value) && isset($value['ti'])) {
+      $arguments['pmep'] = $args[$key];
+    }
+    else {
+      $arguments['menu'][] = $value;

Suppose this code needs a bit of commenting. 

+++ b/plugins/content_types/pm_existing_pages.inc
@@ -29,16 +29,9 @@ function pm_existing_pages_pm_existing_pages_content_type_render($subtype, $conf
+  $args = $panel_args[1];
+  $pm_args = $panel_args[2];

better use same variables name all over code

+++ b/plugins/tasks/pm_existing_pages.inc
@@ -104,7 +109,7 @@ function pm_existing_pages_pm_existing_pages_page() {
-  $output = ctools_context_handler_render($task, $subtask, $contexts, array($context_args[1]));
+  $output = ctools_context_handler_render($task, $subtask, $contexts, array($context_args[1], $menu_args, $pm_args));

let's rename this or above

andypost’s picture

andypost
he/him
Primary language Russian
commented
Status: Needs work » Needs review
StatusFileSize
new 1440068-17.patch4.05 KB

Added check for required arguments so preview does not throw warnings.
Added comment, not sure it's a good to search pmep argument by this way

swentel’s picture

swentel commented
Status: Needs review » Fixed

Went for this one, will test this a very last time at work on monday on a project and then throw out a new release if all goes well.

andypost’s picture

andypost
he/him
Primary language Russian
commented

I seen some todo in doc-block in _pm_existing_pages_get_context_arguments() about _menu_load_objects()

Are you going to refactor this?

swentel’s picture

swentel commented

Hmm, the function actually was passing the wrong arguments, fixed in http://drupalcode.org/project/pm_existing_pages.git/commitdiff/ae2b8a0f8...

Status: Fixed » Closed (fixed)

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