Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom

Display "Professional Services" link in the "An error occured" error screen #9677

Merged
merged 2 commits into from Feb 2, 2016
Merged

Display "Professional Services" link in the "An error occured" error screen #9677

merged 2 commits into from Feb 2, 2016

Conversation

mattab
Copy link
Member

@mattab mattab commented Feb 2, 2016

I can't display the link conditionally as the testMinimumVersion.php file must be PHP4 compatible.

@mattab mattab added the Needs Review PRs that need a code review label Feb 2, 2016
@mattab mattab added this to the 2.16.0 milestone Feb 2, 2016
@tsteur
Copy link
Member

tsteur commented Feb 2, 2016

Can we somehow respect the users settings to disable these ads there? There's a config setting for it but the config might not exist at the time of calling this method. If the class Config is loadable and a config exists it would be nice to respect it.

@mattab
Copy link
Member Author

mattab commented Feb 2, 2016

@tsteur I can't display the link conditionally as the testMinimumVersion.php file must be PHP4 compatible, so we can't use Config or read config file...

mattab pushed a commit that referenced this pull request Feb 2, 2016
Merge pull request #9677 from piwik/error_pro
f7b117e 
Display "Professional Services (Piwik PRO)" link in the "An error occured" error screen
@mattab mattab merged commit f7b117e into master Feb 2, 2016
@mattab mattab deleted the error_pro branch February 2, 2016 22:32
@tsteur
Copy link
Member

tsteur commented Feb 2, 2016

It should work with a class_exists('Piwik\Config', false) I think but would need to be tested

@mattab
Copy link
Member Author

mattab commented Feb 3, 2016

@tsteur because of namespaces it doens't work, I tried:

--- core/testMinimumPhpVersion.php  (revision 888ade738e5e1367406cf7865c5feb5da94ab0fd)
+++ core/testMinimumPhpVersion.php  (revision )
@@ -143,10 +143,15 @@
                             <li><a rel="noreferrer" target="_blank" href="http://piwik.org">Piwik.org homepage</a></li>
                             <li><a rel="noreferrer" target="_blank" href="http://piwik.org/faq/">Piwik Frequently Asked Questions</a></li>
                             <li><a rel="noreferrer" target="_blank" href="http://piwik.org/docs/">Piwik Documentation</a></li>
-                            <li><a rel="noreferrer" target="_blank" href="http://forum.piwik.org/">Piwik Forums</a></li>
-                            <li><a rel="noreferrer" target="_blank" href="https://piwik.pro/contact/?pk_campaign=App_AnErrorOccured&pk_source=Piwik_App&pk_medium=ProfessionalServicesLink#contact-form">Professional help (Piwik PRO)</a></li>
-                            </ul>';
+                            <li><a rel="noreferrer" target="_blank" href="http://forum.piwik.org/">Piwik Forums</a></li>';
+
+            if (class_exists('Piwik\Config', false)
+                && Piwik\Config::getInstance()->General['piwik_pro_ads_enabled'] == 1) {
+                $optionalLinks .= '<li><a rel="noreferrer" target="_blank" href="https://piwik.pro/contact/?pk_campaign=App_AnErrorOccured&pk_source=Piwik_App&pk_medium=ProfessionalServicesLink#contact-form">Professional help (Piwik PRO)</a></li>';
-        }
+            }
+            $optionalLinks .= '</ul>';
+        }
+
         if ($optionalLinkBack) {
             $optionalLinkBack = '<a href="javascript:window.history.back();">Go Back</a>';
         }

but this gives on PHP4

<br />
<b>Warning</b>:  Unexpected character in input:  '\' (ASCII=92) state=1 in <b>[...][...]</b> on line <b>148</b><br />
<br />
<b>Parse error</b>:  syntax error, unexpected T_STRING in <b>[...][...]</b> on line <b>148</b><br />

it can be executed on PHP4 here: http://sandbox.onlinephpfunctions.com/code/1d7b8fa8593da5bcf529ad3282b4acc86122ae9e

so maybe it's not possible because of namespace?

@tsteur
Copy link
Member

tsteur commented Feb 3, 2016 •
edited by mattab

I guess we could do a PHP version check or something but not sure. I guess the way to do it would be to put it in ExceptionHandler. I think it's ok to show when someone tries to install Piwik on PHP 4.X or something and sees the ad. If it comes from the exception handler we do have the config and it would be nice to respect the setting

@mattab mattab changed the title Display "Professional Services (Piwik PRO)" link in the "An error occured" error screen Display "Professional Services" link in the "An error occured" error screen Jul 22, 2016
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers
No reviews
Assignees
No one assigned
Labels
Needs Review PRs that need a code review
Projects
None yet
Milestone
2.16.0
Development

Successfully merging this pull request may close these issues.

None yet
2 participants
@mattab @tsteur