Suitecrm: Remove all PHP warning and use PHP CS

Created on 10 Feb 2016  ·  38Comments  ·  Source: salesagility/SuiteCRM

Hello,

I tried SuiteCRM but it was not possible to install it without having trillion of PHP warning....

"These are just warnings and can be ignored, you can turn off warnings in the php.ini settings." is not an acceptable answer. Because I wanted a CRM quickly up and running I was using the "one click install" of my hosting. SuiteCRM was simply not running with all those warning so I switch to vtiger for the moment.

I think it make normal peoples worried and developer thinking your code is garbage. If you can't fix simple PHP warning can I trust you for other things?

For my point of view the project should use http://cs.sensiolabs.org/ (perhaps with permissive settings) and use it has a first test to reject bad pull request.

EDIT: Warnings was showing up with PHP 5.6 on the installer...

Important Bug

Most helpful comment

Hi Chris,

We've all lost count how many times you have repeated your questions in demand from us here at the team and frankly we have already answered them but you are choosing not to listen and blatantly ignoring those answers. Hence why we have chosen not to repeat those answers in every post you make – waste of our time really and as you know we have more pressing things to address than constantly be on top of a member whom is being more of a hindrance than an assistance at the moment. Just to be clear, lets go over those questions and answers again.

“What are your reasons for why you haven't activated on this github repo, these free standardized and wisely respected code checking tools:”

  1. We lack resources in the product team

We have repeated on a number of occasions that we are a small product team. We are learning and we are shifting our resources where we can and when able – that is how businesses work.
We hadn't and haven't yet pulled these tools in is because we haven't been at the size we are now back in 2013 and even up at 2016. We, as the project owner, need to ensure what the community provides and suggests is a) suitable, b) easily adoptable c) processes put into place that can be extended out to the team, partners and community appropriately.

You're an experienced developer from what you portray so you know that including these tools will have an immediate affect on all of us and that it will more likely generate a lot of failures, issues and need of development to resolve and pass to be suitable for re-release. So you would understand that with the lack of resources we can not take on that challenge lightly. That's where we get to repeating our next answer.

  1. Dedicated time (and resources) to including these tools

In other posts we have already stated that we have taken all the Community feedback. We have listened and will dedicate time in the 7.10 development cycle to improve our development process, including tools and testing. We have come a long way since there has been a dedicate product team.

So yeah we've stated that we are dedicating time and resources in the near future. We respectively ask you to listen to our answers this time. We've stated why we are in the situation and how we approach to resolve it. We want to make the product better and have planned to tackle these issues for 7.10.

All 38 comments

Anyway SuiteCRM should handle error using http://php.net/manual/en/function.set-error-handler.php so error can be logged in database or files instead of being displayed to the end user. Or at least error can be displayed in a nice way without breaking the website.

The other things is http://php.net/manual/en/function.error-reporting.php to be able to set the error we do care about in the backend without having to change php.ini.

So user will have the choice to hide and log error or to display error but in a nice "error box". And the user can chose witch level of error he want to log or display.

On top of that default settings can be the most user friendly and safe by not displaying errors...

:+1:

Although most of these errors are just annoying, some of them seem to actually break the application (I don't fully understand why).

I just tried installing SuiteCRM over XAMPP on Windows and I got this exact behavior as described here:
https://suitecrm.com/forum/installation-upgrade-help/12442-warning-declaration-of-sugaremailaddress-should-be-compatible-with-sugarbean

Note that I had E_STRICT errors turned off.

I removed the offending line (not 1162, which is where the class ends, but near the beginning, where the Save function was declared) and SuiteCRM installed correctly.

It's a pity to lose first-time customers installing SuiteCRM because of this kind of thing. If somebody could explain to me how this kind of issue should be fixed I'd be willing to try.

Maybe this is not just a suggestion, it seems to be a bug (a fatal one) on some circumstances.

Also the errors are displayed all over the page (near each offending block) making the template unreadable.
All errors should be visible only ant the bottom of the page with or without a setting to turn them off.
(Joomla uses that feature so the template has a debug position... and that can also be turned on or off)

There is useful information in #2697 provided by @gunnicom - that bug was closed for being a duplicate of this one, but let's keep it under our sights, there's better detailed technical information there.

Maybe @gunnicom knows enough PHP (I don't) to help me get around the error that breaks the installs on XAMPP:

Warning: Declaration of SugarEmailAddress::save($id, $module, $new_addrs = Array, $primary = '', 
$replyTo = '', $invalid = '', $optOut = '', $in_workflow = false) should be compatible with 
SugarBean::save($check_notify = false) in 
E:\xampp\htdocs\suitecrm\include\SugarEmailAddress\SugarEmailAddress.php on line 1162

The problem is not in line 1162 but above on line 196, where the save function is declared. Notice that the comments already mention this problem... somebody was aware of this, but thought they could get away with it... : - )

Link to that file.

The problem here is, that the definition of SugarEmailAddress::save is really different than the one from the parent class. For most of the other Declaration Warnings its just adding an optional parameter to the definition. For SugarEmailAddress::save its different. Did not look into detail for now.

@pgorod github tip: On the code page if you click the line number you get a direct URL for the lines highlighted (hold shift to select more then one line):
https://github.com/salesagility/SuiteCRM/blob/master/include/SugarEmailAddress/SugarEmailAddress.php#L183-L199

As a quick try maybe it's enough to change the line:
public function save($id, $module, $new_addrs = array(), $primary='', $replyTo='', $invalid='', $optOut='', $in_workflow = false) {

to:
public function save($id, $module=null, $new_addrs = array(), $primary='', $replyTo='', $invalid='', $optOut='', $in_workflow = false) {

That didn't work, but it did work with also changing the id parameter to optional, like this:

public function save($id=null, $module=null, $new_addrs = array(), $primary='', $replyTo='', 
$invalid='', $optOut='', $in_workflow = false) {

SuiteCRM installs cleanly with that change.

Does that look like a safe change to make? Or maybe we should also add code inside save to exit if (!isset($id)) to make up for it?

@gymad provides a solid fix (not just a dodgy workaround) for this in #3018.

This lifted my hopes and made me go out and collect all the places where the fix needs to be applied, see below.

Just for motivation, let me remind here that on some systems these errors show even if E_STRICT errors are turned off (thus creating layout errors visible to end users), and some of these errors seem to be FATAL in some Windows versions of PHP (thus breaking the app). @shogunpol maybe a bug tag is in order for this issue, it can no longer be considered just a suggestion.

Already fixed:

[Wed Jan 04 11:17:13.949763 2017] [:error] [pid 5192:tid 1912] [client ::1:51681] PHP Warning:  Declaration of SugarEmailAddress::save($id, $module, $new_addrs = Array, $primary = '', $replyTo = '', $invalid = '', $optOut = '', $in_workflow = false) should be compatible with SugarBean::save($check_notify = false) in C:\\xampp\\htdocs\\SuiteCRM\\include\\SugarEmailAddress\\SugarEmailAddress.php on line 1162, referer: http://localhost/SuiteCRM/install.php

The process function, to be fixed for ALL dashlets, only a sample shown here (the ones used by default on the home screen):

[Wed Jan 04 11:25:07.565361 2017] [:error] [pid 5192:tid 1892] [client ::1:51716] PHP Warning:  Declaration of MyCallsDashlet::process($lvsParams = Array) should be compatible with DashletGeneric::process($lvsParams = Array, $id = NULL) in C:\\xampp\\htdocs\\SuiteCRM\\modules\\Calls\\Dashlets\\MyCallsDashlet\\MyCallsDashlet.php on line 47, referer: http://localhost/SuiteCRM/index.php?action=Login&module=Users

[Wed Jan 04 11:25:07.784111 2017] [:error] [pid 5192:tid 1892] [client ::1:51716] PHP Warning:  Declaration of MyMeetingsDashlet::process($lvsParams = Array) should be compatible with DashletGeneric::process($lvsParams = Array, $id = NULL) in C:\\xampp\\htdocs\\SuiteCRM\\modules\\Meetings\\Dashlets\\MyMeetingsDashlet\\MyMeetingsDashlet.php on line 209, referer: http://localhost/SuiteCRM/index.php?action=Login&module=Users

[Wed Jan 04 11:25:07.799736 2017] [:error] [pid 5192:tid 1892] [client ::1:51716] PHP Warning:  Declaration of MyOpportunitiesDashlet::process($lvsParams = Array) should be compatible with DashletGeneric::process($lvsParams = Array, $id = NULL) in C:\\xampp\\htdocs\\SuiteCRM\\modules\\Opportunities\\Dashlets\\MyOpportunitiesDashlet\\MyOpportunitiesDashlet.php on line 90, referer: http://localhost/SuiteCRM/index.php?action=Login&module=Users

[Wed Jan 04 11:25:07.846613 2017] [:error] [pid 5192:tid 1892] [client ::1:51716] PHP Warning:  Declaration of MyAccountsDashlet::process($lvsParams = Array) should be compatible with DashletGeneric::process($lvsParams = Array, $id = NULL) in C:\\xampp\\htdocs\\SuiteCRM\\modules\\Accounts\\Dashlets\\MyAccountsDashlet\\MyAccountsDashlet.php on line 101, referer: http://localhost/SuiteCRM/index.php?action=Login&module=Users

[Wed Jan 04 11:25:07.895362 2017] [:error] [pid 5192:tid 1892] [client ::1:51716] PHP Warning:  Declaration of SugarFeedDashlet::process($lvsParams = Array) should be compatible with DashletGeneric::process($lvsParams = Array, $id = NULL) in C:\\xampp\\htdocs\\SuiteCRM\\modules\\SugarFeed\\Dashlets\\SugarFeedDashlet\\SugarFeedDashlet.php on line 607, referer: http://localhost/SuiteCRM/index.php?action=Login&module=Users

The setup function:

[Sun Jan 08 04:15:59.868861 2017] [:error] [pid 6048:tid 1900] [client ::1:50251] PHP Warning:  Declaration of PopupSmarty::setup($seed, $file = NULL, $where = NULL, $params = Array, $offset = 0, $limit = -1, $filter_fields = Array, $id_field = 'id') should be compatible with ListViewDisplay::setup($seed, $file, $where, $params = Array, $offset = 0, $limit = -1, $filter_fields = Array, $id_field = 'id', $id = NULL) in C:\\xampp\\htdocs\\SuiteCRM\\include\\Popups\\PopupSmarty.php on line 46, referer: http://localhost/SuiteCRM/index.php?action=DetailView&module=ProspectLists&record=31be6f73-7d2a-82bb-ceac-5872252497d6&return_module=ProspectLists&return_action=DetailView&offset=1

The ACLAccess function:

[Wed Jan 04 11:24:14.341216 2017] [:error] [pid 5192:tid 1908] [client ::1:51689] PHP Warning:  Declaration of AOR_Report::ACLAccess($view, $is_owner, $in_group, $target_module) should be compatible with SugarBean::ACLAccess($view, $is_owner = 'not_set', $in_group = 'not_set') in C:\\xampp\\htdocs\\SuiteCRM\\modules\\AOR_Reports\\AOR_Report.php on line 1541, referer: http://localhost/SuiteCRM/install.php

And a few I copied from @gunnicom at #2697:

Declaration of SugarFeedDashlet::saveOptions($req) should be compatible with DashletGeneric::saveOptions(array $req) in /var/www/sugar/modules/SugarFeed/Dashlets/SugarFeedDashlet/SugarFeedDashlet.php

Declaration of SugarWidgetFielduser_name::displayInput(&$layout_def) should be compatible with SugarWidgetFieldVarchar::displayInput($layout_def) in /var/www/sugar/include/generic/SugarWidgets/SugarWidgetFielduser_name.php

Questions:
Do any of you know of more places where these same errors appear?
Or other errors of the same kind?
And to save us some guessing work, can a compiler find them?

Has anyone here tried to install and run the command line php coding standards fixer.
composer global require friendsofphp/php-cs-fixer

Or the plugin:
http://tzfrs.de/2015/01/automatically-format-code-to-match-psr-standards-with-phpstorm

I tried https://github.com/friendsofphp/php-cs-fixer and it works great.
To run it on the entire codebase for the purposes of fixing the coding standards, would obviously cause a massive PR containing every single PHP file in the application, updated to fix all the PSR coding standards problems in practically all the PHP files.

There is also Code Sniffer: https://github.com/squizlabs/PHP_CodeSniffer that has a configurable set of rules to check and includes a fixer script: phpcbf that can automatically correct some of the errors found.

I've created a Code sniffer ruleset: https://github.com/pribeiro42/SuiteCS according to the wiki page on https://suitecrm.com/wiki/index.php/Coding_Standards if you want to try it...

Of course, the final result is the same: if you run any of those tools on all of the codebase all php files will change :)

@samus-aran this one is also a bug, as per my 1 Fev comment. People install SuiteCRM and it's immediately broken - luckily this only happens on a few Windows configurations, but still...

I run the squizlabs PHP_CodeSniffer on a google written php library. Zero fatal errors, zero warnings, zero deprecated, zero strict.

SuiteCRM needs to achieve this same level of php code quality. There should be no warnings or fatal errors, on all supported php versions (ideally all current stable releases of php which currently is 5.6, 7.0 and 7.1). Anything lesser level of quality makes users confidence erode and you risk losing them forever.

To get there:

  • There should be established a team devoted to eradicating all warnings, strict, deprecated, fatal errors, for the current stable versions of php.
  • A team devoted to converting to composer everywhere.
  • A team devoted to accumulating phpunit tests, Travis CI, etc.
  • Every merge should only work when it fully passes travis-ci and fail when the automated tools fail on any unit test.

People keep stumbling into this and it makes SuiteCRM look bad. Apart from this issue referenced above, 7 days ago, today another one came up in the forums:

https://suitecrm.com/forum/installation-upgrade-help/14014-errors-after-upgrading

This is low-hanging fruit, it could be easily fixed even by a very junior developer and merged without much concern.

The question goes to @samus-aran @willrennie @salesagility @mattlorimer
It has been over one year.
What are your reasons why you have not activated on this repo:

  1. php-cs-fixer,
  2. scrutinizer-ci,
  3. PHP_CodeSniffer
    ...to automatically flag all of the embarrassing errors and warnings that have been laying around in the code since 2004, automatically provide suggested fixes, and improve the quality of the project.

Chris I'm afraid there are probably no good technical reasons not to, but they're simply too busy doing the work that pays their salaries. I think that's understandable. In the long run, it's also in our interest that they do that.

They also do a lot of work here, it might seem it's not enough, but who are we to complain? There's only so much pressure we can put on these guys, the rest we just have to wait and let them have their rhythm even if it seems slow to us.

There's also something called risk. They're taking a lot of risk already with SuiteCRM (a relatively small company, hiring people for a job that only very indirectly generates income), so many of these bold technical moves feel too risky for them. Anyway it's not all the same, the ones mentioned on this thread would actually lower risks, and I think eventually we will see SalesAgility applying them.

I think nobody more than SalesAgility devs feels the frustration when some piece of stupid defective code they inherited bites them on some Issue. They would probably love to use all these tools and frameworks but the truth is that each of them would require careful consideration, experimentation, perfecting processes etc., and that takes time and demands peace of mind, which might be hard to come by at the moment...

Let's not make excuses for them, and let them speak for themselves.

Just installed 7.9.1 on

$ uname -a
Linux (redacted host name) 3.10.0-514.16.1.el7.x86_64 #1 SMP Wed Apr 12 15:04:24 UTC 2017 x86_64 x86_64 x86_64 GNU/Linux

and am getting

Warning: Declaration of MyCallsDashlet::process($lvsParams = Array) should be compatible with DashletGeneric::process($lvsParams = Array, $id = NULL) in <install_dir>/modules/Calls/Dashlets/MyCallsDashlet/MyCallsDashlet.php on line 47

Warning: Declaration of MyMeetingsDashlet::process($lvsParams = Array) should be compatible with DashletGeneric::process($lvsParams = Array, $id = NULL) in <install_dir>/modules/Meetings/Dashlets/MyMeetingsDashlet/MyMeetingsDashlet.php on line 209

Warning: Declaration of MyOpportunitiesDashlet::process($lvsParams = Array) should be compatible with DashletGeneric::process($lvsParams = Array, $id = NULL) in <install_dir>/modules/Opportunities/Dashlets/MyOpportunitiesDashlet/MyOpportunitiesDashlet.php on line 90

Warning: Declaration of MyAccountsDashlet::process($lvsParams = Array) should be compatible with DashletGeneric::process($lvsParams = Array, $id = NULL) in <install_dir>/modules/Accounts/Dashlets/MyAccountsDashlet/MyAccountsDashlet.php on line 101

Warning: Declaration of SugarFeedDashlet::process($lvsParams = Array) should be compatible with DashletGeneric::process($lvsParams = Array, $id = NULL) in <install_dir>/modules/SugarFeed/Dashlets/SugarFeedDashlet/SugarFeedDashlet.php on line 607

Any recommendations?

@ChipNowacek

If you are getting those errors on screen, with a working, app, try simply turning off display_errors in php.ini. You might also set error_reporting=E_ALL & ~E_DEPRECATED & ~E_STRICT. Then restart Apache.

If you are getting those errors in the logs, with a working app, you can safely ignore them.

If you are getting those errors in the logs, with a broken SuiteCRM, then change each file to add a second parameter as optional in the definition of the process function:

process($lvsParams = Array, $id = NULL)

Note that this function definition is not found in the line given in the error, but in an earlier line of the same file.

Some people have worked around this by upgrading PHP version, sometimes it helps. If this is a good time to switch to PHP 7, go ahead...

@ChipNowacek You're right, these warnings are from sloppy php programming from back at the start of SugarCRM in 2004. They're filling up the logs have led to numerous justifiable complaints for more than a year, see also #2697 #3279 among many others. They're being gradually fixed but the pace needs to speed up to much faster than glacial.

If you would like to fix the missing parameters in the code you can switch to the Hotfix branch navigate to the dashlet code files and click the pencil icon, edit, save, then create your own PR, maybe it will get merged in 2 days or 2 years, it's somewhat unpredictable.

@chris001 @pgorod Thank you for responding.

I am running PHP Version 7.0.17 on a shared server. I am opening a ticket with the host asking if I can add an additional php.ini file and, if necessary, to restart Apache.

I can see how this would be discouraging for you guys.

Just got this from my host. Does this make sense to you?

Hello,

You can add one there, as long as the directives work in the directory context. No restart will be required, as they are CGI processes.

I really don't know much about shared hostings, I avoid them as best as I can. But I've seen cases where you just drop a php.ini in a folder and they somehow take it into account, for that area of the folder tree. I guess that's what they mean in that message.

So try playing with additional php.ini files. You can go into Admin/diagnostics to get a phpinfo output that will help you track exactly what directives are taking effect or not.

Or you can simply put this script in your SuiteCRM root folder and call it phpinfo and then invoke it from a browser with http://www.mydomain.com/phpinfo:

<?php
     phpinfo();
?>

Got rid of the warnings with local php.ini:

display_errors = 0
error_reporting=E_ALL & ~E_DEPRECATED & ~E_STRICT

If I stay subscribed to this issue, will I see the fix? I'd like to remove error suppression when I can.

@ChipNowacek Yes you'll get the notifcations

Thanks for your help.

By the way, I started the fix on my fork, but I haven't finished it yet, and I haven't made the Pull Request.

The part that is done is exactly the part affecting you, @ChipNowacek. It's very easy to do manually if you want, see the changes here.

The other problematic functions (see my list above) I hesitated on how exactly I should fix them (for example ACLAccess) and then I ran out of time and abandoned this :-) hopefully I'll get back to it soon.

Thank you for the fix info. Helpful. I'll make the changes on my install.

The question goes to @samus-aran @willrennie @salesagility @mattlorimer @gregsoper
Please - no one else try to speak for, or speculate on behalf of, the actual @salesagility employees - shareholders.
It's been nearly 18 months.
What are your reasons for why you haven't activated on this github repo, these free standardized and wisely respected code checking tools:

  • php-cs-fixer,
  • scrutinizer-ci,
  • PHP_CodeSniffer which @gagarine asked for and which is a mainstay of almost all top notch PHP projects,
...to automatically report here on github for all contributors to see, detailed recommended fixes for all of the errors and warnings that have been laying around in the sugar code since 2004, for the purpose of making it more efficient, time-effective and clear what contributors can do to improve the quality of the existing product code and their new code.

Thank you. Looking forward to your direct replies and opening up a dialogue on this topic.

Hi Chris,

We've all lost count how many times you have repeated your questions in demand from us here at the team and frankly we have already answered them but you are choosing not to listen and blatantly ignoring those answers. Hence why we have chosen not to repeat those answers in every post you make – waste of our time really and as you know we have more pressing things to address than constantly be on top of a member whom is being more of a hindrance than an assistance at the moment. Just to be clear, lets go over those questions and answers again.

“What are your reasons for why you haven't activated on this github repo, these free standardized and wisely respected code checking tools:”

  1. We lack resources in the product team

We have repeated on a number of occasions that we are a small product team. We are learning and we are shifting our resources where we can and when able – that is how businesses work.
We hadn't and haven't yet pulled these tools in is because we haven't been at the size we are now back in 2013 and even up at 2016. We, as the project owner, need to ensure what the community provides and suggests is a) suitable, b) easily adoptable c) processes put into place that can be extended out to the team, partners and community appropriately.

You're an experienced developer from what you portray so you know that including these tools will have an immediate affect on all of us and that it will more likely generate a lot of failures, issues and need of development to resolve and pass to be suitable for re-release. So you would understand that with the lack of resources we can not take on that challenge lightly. That's where we get to repeating our next answer.

  1. Dedicated time (and resources) to including these tools

In other posts we have already stated that we have taken all the Community feedback. We have listened and will dedicate time in the 7.10 development cycle to improve our development process, including tools and testing. We have come a long way since there has been a dedicate product team.

So yeah we've stated that we are dedicating time and resources in the near future. We respectively ask you to listen to our answers this time. We've stated why we are in the situation and how we approach to resolve it. We want to make the product better and have planned to tackle these issues for 7.10.

Thank you very much Ashely @samus-aran for your detailed reply. Assuming you're more or less speaking with Daniel Dillon and Gyula who added joyful emoticons to your post.

Trust me on this, we don't enjoy pointing out areas that seriously need strengthening. It doesn't give us any joy. Most of these areas are legacy Sugar code anyway so you shouldn't take it personally since you didn't write it. The intention however is to help by adding voices to show where are the areas which need serious attention in order to sooner rather than later result in a massive uplift to the quality of the app, and eventually, to allow it to work more or less flawlessly for a vastly greater number of users on the planet and maybe eventually on other planets in the solar system.

As is traditional, it's our duty and responsibility to point out where improvements can be made and should be made, until you can come back with either an acceptable reasonable response of how things are being remedied, or a good enough reason why they can't or you feel shouldn't be. This is how open source works, with open communications. Far too many times have great Pull Requests and totally awesome Issues been filed here and gone literally years with no action. O.O This very issue right here is just one of them. It makes no sense. It shouldn't be that way. This issue (adding php_cs) is just one of the many designed to save time, after a small initial investment of time, then forever after, everyone's time will be massively saved.

Anyway, it appears there's two competing interests going on here - your commercial work for clients on the one hand which is wonderful and to be applauded, and pure product improvement on the other hand which is equally fabulous yet shouldn't be controlled entirely by Sales Agility. Let's do some back of the envelop calculations. 32 engineers, architects, business analysts and support engineers in the company, times 40 hours per week, equals 1280 tech-person-hours per week. To help give us all an idea of how much product improvement we the community could reasonably expect in any given time frame, let me ask on behalf of the entire community: How many tech-person-hours per week are devoted to product? Thanks Ashley. 👍

Ok, I'm happy to see a more positive approach to this Chris, we appreciate that. We are always aware of the frustrations of the community and completely understand them – we have them too. So we hope that you now understand that we are learning how to best utilise what we have and provide a suitable and consistent standard of the product.

In your calculation you had included non technical employees. Greg had stated in another post of his that we have a team of 5 persons dedicated to the product. They include a number of roles – development, support and community. This is why we have encouraged in the past both developers and testers from the community to assist with releases and ongoing bug fixes.

Yes we do have a commitment to clients, but also to the product itself. To keep an Open Source project open it requires finance and that is a dedicated task itself. Just want to bring up a quote you made in the other post:

“Funding I'm stunned you're asking about funding at this late date. I thought SA was financially in top shape with no financial need toward product development, and were funding the product development out of profits from commercial support income.”

Just want to make things clear - we are in top shape. We are stating that maintaining an Open Source product is difficult regardless what size you are. We all know what happened to SugarCRM CE.... a prime example. We do not want to go down that route, but not doing so requires time. I hope you have now corrected your view on how we are not 'money-making' machine and only care about our 'rich clients'. How can we not take things personally if that is the kind of incorrect accusations that are being thrown at us?

So as I said previously we have dedicated that time coming soon when 7.9 is stabilised and I believe – along with my fellow product team (@gymad, @Dillon-Brown , @daniel-samson ) that we will smooth away our frustrations and massively save time in the long run but we need you @chris001 , specifically you, to be patient with us and assist.

  • Ashley

@samus-aran still implementing http://php.net/manual/en/function.set-error-handler.php is a 1h job and will fix the critical issue of the product simply not working because error are printed all over the place.

Here's a new one

Strict Standards: Non-static method Audit::get_audit_list() should not be called statically, assuming $this from incompatible context in /crm/modules/Audit/Popup_picker.php on line 104

I'm afraid I don't know the correct way to fix this... maybe one of our PHP experts can advise?

And a new one to keep us busy

Warning: Declaration of ViewProperty::init() should be compatible with SugarView::init($bean = NULL, $view_object_map = Array) in /modules/ModuleBuilder/views/view.property.php on line 150

Here's a handy link to the file.

It seems this happens when you attempt to make layout changes to the Accounts module in Studio.

I raised all these code quality issues several years ago on the forum, and was told from the top that the code base is not in any shape or form the issue for getting SuiteCRM accepted and used by the community. I disagreed, and my thread was promptly deleted.

Moving to composer and other modern ways of writing PHP and structuring the code, is essential IMO to move the product on. The code base need to feel right, and be consistent and intuitive, or people just won't be able to dive in and help in any productive way. There is so much legacy crap going back many, many years that needs to be addressed so much. The way it is at the moment leaves SuiteCRM in the security-nightmare hole that SugarCRM has always been in.

Other bugbears of mine:

  • Mixed tabs and spaces everywhere!
  • PHP4 object definitions using var and suchlike. That stuff, soon, will simply stop working on PHP 7.x when PHP says, "enough is enough, move on already".
  • Namespaces - please, please, please.
  • composer - so much exists out there that can be reused here; don't keep reinventing the wheel.
  • autoloader - there are over 400 require() statements and 6500 - read that again - 6500 require_once() statements in the code base. In 2017. Six thousand five hundred. I mean, WTF, how smelly is that?
  • so many entry points - all that code is open to the whole Internet, and just waiting for exploits to be used against it. Only the public entry points should be available to the, err, public. Everything else need to be moved outside of the public root. Well outside. Out of direct reach. Just, gone.

Anyway, I've hijacked this issue enough, so I'll get back to fixing these things just to get the latest stable release working on my server.

Was this page helpful?
0 / 5 - 0 ratings