Suitecrm: Non-static method MBModule::getTypes() should not be called statically

Created on 18 Aug 2017  路  11Comments  路  Source: salesagility/SuiteCRM


Issue

Legacy code structural issues.

Expected Behavior

No errors

Actual Behavior

Deprecated: Non-static method MBModule::getTypes() should not be called statically in /.../modules/ModuleBuilder/views/view.module.php on line 74

Possible Fix

Not yet looked

Steps to Reproduce

  1. Go to module builder and try to create a module.
    2.
    3.
    4.

Context

This is one of those things that are rife in this code base, and there have been static tools available for years to identify them for fixing.

Your Environment

  • SuiteCRM Version used: 7.9.4
  • Browser name and version (e.g. Chrome Version 51.0.2704.63 (64-bit)): n/a
  • Environment name and version (e.g. MySQL, PHP 7): PHP7.1
  • Operating System and version (e.g Ubuntu 16.04): CentOS
Moderate Fix Proposed Bug

Most helpful comment

Keep 'em small to help with that. If there is a pile of formatting and comments to change, then keep them all in a single PR that can be merged quickly and confidently to get it out of the way. Then the functional changes will be easier to track in a separate PR.

I'm not an official member of this project, but have seen the way these massive PRs can go on other projects.

All 11 comments

The function MBModule::getTypes() needs to be declared static. It may help to declare any method that is truly static as static across the board.

https://github.com/salesagility/SuiteCRM/blob/master/modules/ModuleBuilder/MB/MBModule.php#L627

Created PR with this tiny-weeny change. The file is a mess of tab and space indents, like much of the code base, and I would like to come back and fix that another day. In the meantime, I'm just working on getting one installed, stable product up and working.

Many other errors in the module builder that is making it unusable:

Warning: Declaration of ManyToOneRelationship::setReadonly() should be compatible with AbstractRelationship::setReadonly($set = true) in /.../modules/ModuleBuilder/parsers/relationships/ManyToOneRelationship.php on line 0

Warning: Declaration of ManyToOneRelationship::buildLabels() should be compatible with AbstractRelationship::buildLabels($update = false) in /.../modules/ModuleBuilder/parsers/relationships/ManyToOneRelationship.php on line 0

And various other methods that don't match the abstract definitions. Every time I fix one, another pops up. This whole module needs to be tided up properly.

I believe this related to my pull request #4097.

@daniel-samson is that a virtually re-written Module Builder I see? I can help test it if it is as simple as extracting that one module to my dev instance.

It is crazy that all those modules are in one giant repo, and not in repos of their own that can be developed and installed independently.

@judgej I welcome more eyeballs on the pull request. The more the merrier.

Keep 'em small to help with that. If there is a pile of formatting and comments to change, then keep them all in a single PR that can be merged quickly and confidently to get it out of the way. Then the functional changes will be easier to track in a separate PR.

I'm not an official member of this project, but have seen the way these massive PRs can go on other projects.

@daniel-samson How would I get hold of your PR to try locally? github seems to have made it very difficult to identify just which branch this is. I've tried your master branch, but can't see the changes in there.

@judgej You can see which branch a PR has been made from slightly below the title. In this case #4097 was made from: feature_module_builder_with_tests

Thanks. github certainly needs more clickable links to make this stuff easier.

Module builder is broken for me because of this error. Any ETA on the fix?

image

Was this page helpful?
0 / 5 - 0 ratings