Skip to content
This repository has been archived by the owner on Mar 8, 2021. It is now read-only.

if move site with phx then have bug and we are can't set default path #1017

Closed
Dmi3yy opened this issue Nov 30, 2016 · 33 comments
Closed

if move site with phx then have bug and we are can't set default path #1017

Dmi3yy opened this issue Nov 30, 2016 · 33 comments
Labels

Comments

@Dmi3yy
Copy link
Collaborator

Dmi3yy commented Nov 30, 2016

http://take.ms/RX1Vi

or if none phx file.

@Dmi3yy Dmi3yy added the 1.2.1 label Nov 30, 2016
@modxuser
Copy link

I didn't realise that that was a bug

I had that each time I moved from localhost to live server, as the path to PHX was different. I always logged into the manager and reset the paths and all was then OK.

But, I no-longer use PHX so don't have that problem anymore LOL

@Dmi3yy
Copy link
Collaborator Author

Dmi3yy commented Nov 30, 2016 via email

@MrSwed
Copy link
Contributor

MrSwed commented Nov 30, 2016

  1. disable phx
  2. correct paths
  3. enable phx

@Dmi3yy
Copy link
Collaborator Author

Dmi3yy commented Nov 30, 2016 via email

@MrSwed
Copy link
Contributor

MrSwed commented Dec 5, 2016

here
https://github.com/modxcms/evolution/blob/develop/manager/actions/mutate_settings/tab1_site_settings.inc.php#L137

need catch error..
or check path befor use any invoke events in configs

@MrSwed
Copy link
Contributor

MrSwed commented Dec 5, 2016

#623 (reference)

@yama
Copy link
Collaborator

yama commented Dec 5, 2016

if(is_file(MODX_BASE_PATH . 'assets/plugins/phx/phx.class.inc.php'))

maybe add this

@Dmi3yy
Copy link
Collaborator Author

Dmi3yy commented Dec 5, 2016 via email

@yama
Copy link
Collaborator

yama commented Dec 5, 2016

It should be same error on the front page, how is it?

@Dmi3yy
Copy link
Collaborator Author

Dmi3yy commented Dec 5, 2016 via email

@yama
Copy link
Collaborator

yama commented Dec 5, 2016

a
hmm...? I do not know well, but it is certainly strange

@MrSwed
Copy link
Contributor

MrSwed commented Dec 5, 2016

I think I need a more accurate verification decision path in general, not specifically relating phx. If the check does not pass, first allow the user to adjust them, implying that the site has been moved

@Dmi3yy
Copy link
Collaborator Author

Dmi3yy commented Dec 5, 2016 via email

@MrSwed
Copy link
Contributor

MrSwed commented Dec 5, 2016

@yama

if(is_file(MODX_BASE_PATH . 'assets/plugins/phx/phx.class.inc.php'))

no #623 (reference) was after two tryes, because phx can be renamed, including rename plugin files...

@MrSwed
Copy link
Contributor

MrSwed commented Dec 5, 2016

@Dmi3yy
there are is error occurs
$modx->invokeEvent('OnParseDocument')
before you test path to phx :)

@MrSwed
Copy link
Contributor

MrSwed commented Dec 5, 2016

something like (proposed logic)

if(!is_file(MODX_BASE_PATH . 'manager/include/config.inc.php')) {
 // promt to correct MODX_BASE_PATH
} else {
// existing output configuration.
}

?

@yama
Copy link
Collaborator

yama commented Dec 5, 2016

but can’t set by default base_path in settings.

hmm...?

@MrSwed
Copy link
Contributor

MrSwed commented Dec 5, 2016

@yama #1017 (comment)

MrSwed referenced this issue in Dmi3yy/modx.evo.custom Dec 5, 2016
@Deesen
Copy link
Contributor

Deesen commented Dec 5, 2016

Or maybe replacing invokeEvent('OnParseDocument') with check for phx-plugincode and disabled != 1

// Check if PHX is enabled
$count = $modx->db->getRecordCount(
  $modx->db->select('id', $modx->getFullTableName('site_plugins'), 
  "plugincode LIKE '%phx.parser.class.inc.php%OnParseDocument();%' AND disabled != 1")
);

if($count) {
    $disabledFilters = 1;
    echo '<b>'.$_lang['enable_filter_phx_warning'].'</b><br/>';
}

@yama
Copy link
Collaborator

yama commented Dec 5, 2016

its better

@yama
Copy link
Collaborator

yama commented Dec 5, 2016

https://github.com/extras-evolution/PHx/blob/master/assets/plugins/phx/phx.plugin.txt#L15
The old version of PHx referred to "rb_base_dir". Is that problem?

@Deesen
Copy link
Contributor

Deesen commented Dec 5, 2016

In my testing-case yes.

@yama
Copy link
Collaborator

yama commented Dec 5, 2016

ok, understood. I will fix this issue later

@yama
Copy link
Collaborator

yama commented Dec 5, 2016

// Check if PHX is enabled
$count = $modx->db->getRecordCount(
  $modx->db->select('id', $modx->getFullTableName('site_plugins'), 
  "plugincode LIKE '%phx.parser.class.inc.php%OnParseDocument();%' AND disabled != 1")
);

if($count) {
    $disabledFilters = 1;
    echo '<b>'.$_lang['enable_filter_phx_warning'].'</b><br/>';
}

I thought carefully, I think this fix is smarter.
I think that rb_base_dir issue needs to be solved as another problem.

@MrSwed
Copy link
Contributor

MrSwed commented Dec 6, 2016

may be better

"plugincode LIKE '%PHxParser%OnParseDocument();%' 

?

@Dmi3yy
Copy link
Collaborator Author

Dmi3yy commented Dec 6, 2016 via email

@Deesen
Copy link
Contributor

Deesen commented Dec 6, 2016

@Dmi3yy But what is you exact error-message? For me the error is

  • invokeEvent onParseDocument
  • this will eval() plugin-code of PHX
  • plugincode starts with include_once ($modx->config['rb_base_dir']), not found
  • calling $PHx->OnParseDocument() will cause fatal-error

So avoid invokeEvent should avoid any incorrect base-path at least for PHx? Or do other plugins cause problems OnParseDocument too?

@yama
Copy link
Collaborator

yama commented Dec 6, 2016

But this check not work with incorect base_parh

Is it "base_path"? not "rb_base_dir"?

@MrSwed
Copy link
Contributor

MrSwed commented Dec 6, 2016

updated phx has correct path
and I yesterday update site with old phx (rb_base_dir) - phx still work correctly

@Deesen
Copy link
Contributor

Deesen commented Dec 6, 2016

Maybe this is a more global solution to check all kinds of plugins? #1060 (comment)

@dmitryy
Copy link

dmitryy commented Dec 6, 2016

@Deesen please write correct @Dmi3yy not @dmitryy ;)

@Deesen
Copy link
Contributor

Deesen commented Dec 6, 2016

Sry :-D

@MrSwed
Copy link
Contributor

MrSwed commented Dec 6, 2016

@Deesen
first - check correct path
second - check path fo each plugin befor run/invokeevent

@Dmi3yy Dmi3yy closed this as completed Dec 8, 2016
Dmi3yy added a commit that referenced this issue Nov 25, 2019
Changed icons to more understandable
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
Projects
None yet
Development

No branches or pull requests

6 participants