Revolution: Collect deprecated methods/classes to be removed in 3.0

Created on 27 Nov 2016  路  40Comments  路  Source: modxcms/revolution

Summary

Collect deprecated methods/classes to be removed in 3.0 in the MODX core code and add logging code in those methods/classes.

This should be done before the release of 2.6 to have some warning time for extra authors.

This issue could be used to collect those methods/classes. Please add the methods/classes with the filename, classname and method name in a new comment.

Filename|Classname|Method
---------|-----------|--------
core/model/modx/rest/modrestclient.class.php|modRestClient|-
core/model/modx/rest/modrestcurlclient.class.php|modRestCurlClient|-
core/model/modx/rest/modrestserver.class.php|modRestServer|-
core/model/modx/rest/modrestsockclient.class.php|modRestSockClient|-
core/xpdo/xpdo.class.php|xPDO|toJson
core/xpdo/xpdo.class.php|xPDO|fromJson

modRestClient is used in core for Package Management. So the core methods have to be changed to use modRestService or the deprecated notice has to be removed.

Unsure:

Filename|Classname|Method|Reason
---------|-----------|--------|-------
core/model/modx/rest/modrest.class.php|modRest|-|Is it replaced by modRestService?

area-core proposal

Most helpful comment

Having two "deprecated" errors every time you use the Installer is just annoying, and embarrassing that it should still be an issue after four years. Is there someone, anyone, who can look at it and see if, at the least, the errors can be removed? Are they actually not working correctly? Are they actually going to be removed in MODX 3?

All 40 comments

I didn't realise the modRest utils were going to be removed? Got a source for that, or are you just proposing we do?

All the classes mentioned above have a line like this:
https://github.com/modxcms/revolution/blob/2.x/core/model/modx/rest/modrestclient.class.php#L11

modRestService, modRestController etc. are not touched by this.

Ah.. don't think I noticed that before :D Makes me wonder what other code we have that is marked as deprecated in that way from years ago, and how we feel about those things today. Some deprecations will be logical as things are refactored, but there might also be things that we want to take out just because it's not something MODX should provide.

The classes of the first table seemed to be improved a lot in modRestService, so they could be deprecated. But I have no code using these classes, so I am maybe wrong.

modRestClient is used in core.

According to @theboxer the S3 Media Source code in the core can't be upgraded to the newest S3 API. So that part should be marked as deprecated.

Can't as in the S3 Media Source library dependency has changed, or some other reason? Does the S3 Media Source require some adjustments or a rewrite?

There is an extra package available: https://github.com/modxcms/aws-s3-media-source

@opengeek mentioned on the MAB call that he had a list of "hundreds" deprecations.

Consider adding $modx->toJSON to the list. Can't recall when I've seen PHP withiut JSON enabled by default.

I agree @whitebyte . That method made sense in the past, but no longer.

The ability to use flat file processors (2.1-style) should also be removed in 3.0, in favour of the class based processors only. I believe that logic is located in the modConnectorRequest and/or modConnectorResponse class.

System settings to be removed:

  • filemanager_url
  • filemanager_url_relative

screen shot 2018-04-23 at 11 58 23

Are we still going to remove those in 3.x?

System setting to remove "server_protocol" #13269

14057 removes the legacy contentType (not content_type), should be documented/marked as deprecated in 2.7.

I guess modparser095.class.php yet one candidate for deleting in 3.0

I'd be in favour of removing that as well. It probably makes sense to also remove modTranslate095 and modTranslator in that case, which is the only core usage of modParser095 and designed as a one-time conversion utility. The only usage of modParser095 that I can find is Provisioner, which could be updated to include its own copy of the parser class if there's still demand for that.

modClassMap is deprecated with 2.2. I am not sure, if that could be logged. Would $xpdo contain $modx methods in __construct?

14252 removes upload_flash settings

Proposing the removal of "Import HTML" and "Import Static Resources". The functionality of these two pages does not need to be a part of the MODX core. They are not used _very_ often and should be pretty easy to split out as separate extras, as they are (more or less) completely isolated from the rest of the core. I do not think anything depends on these classes.

I used "import HTML" once because it was there, but I would have been better off with a more customizable add-on, since it left me with the need for a lot of small queries to manipulate what got imported.

"Role" in "Access Control Lists". In my opinion, it is not used by anyone, and it is not clear how it works :)

That certainly wouldn't break my heart!

"Role" in "Access Control Lists". In my opinion, it is not used by anyone, and it is not clear how it works :)

Why do you think it's not being used by anyone? There are a lot of features in MODX you and I might not use because we don't know about them or don't understand them, that doesn't mean they are not being used.

See for more info about Roles the docs.

Well, it would take a complete reworking of the entire permissions system, including all the permissions checks to get rid of them, so it's not really worth the bother. Easy enough to simply ignore them, or else give everybody 0. Then those who do use them can continue to happily do so.

In some ways, they resemble contexts in that regard. Except for situations like multiple domains using a single Manager, you can get the basic resource structure to do anything that contexts can be used for.

"Recent Resources" tab in the "Account" section.

recent_resources

This tab is superfluous in my opinion. Firstly, there is a widget on the "Home" section, and secondly, there is a section "Manager log" to which, by the way, the widget refers on the "Home" section.

"Recent Resources" tab in the "Account" section.

But in the widget general information, when here only related to this specific user.

But in the widget general information...

It's strange, is access control taken into account in the widget? For example, some user should not see some resources, but he will see all of them in the widget? And can he start editing? The same question to the "Manager log" section, there is a filter by users.

... when here only related to this specific user.

If it works like that, then there is logic in it. Although, I thought that only the current user is shown in the widget.

In both cases, it calls GetRecentlyEditedResources processor with limit by the current user.

Flash in the manager. Not a method / class, but it needs to be added to the list for removal.

Flash in the manager. Not a method / class, but it needs to be added to the list for removal.

Can you be more specific? I believe the swf files are removed already.

/manager/assets/ext3/resources {expressinstall.swf, charts.swf}
/core/packages/core/MODX/Revolution

Also in the packages:
/core/packages/core/MODX/Revolution/modContext/e74fa3f7bc1acb8b732de8e70427c6e4/0/assets/ext3/resources

The affects MODX 2 and MODX 3. Firefox throws a warning (in the address bar) to use the outdated version of Flash. As I do not have Flash installed.

I believe they are called in /manager/assets/ext3/ {ext-all-debug.js, exct-all.js} :

Ext.chart.Chart.CHART_URL = 'http:/' + '/yui.yahooapis.com/2.8.2/build/charts/assets/charts.swf';
...

Ext.FlashComponent.EXPRESS_INSTALL_URL = 'http:/' + '/swfobject.googlecode.com/svn/trunk/swfobject/expressInstall.swf';

Good catch, those are ExtJS dependencies and I'm not sure if we can remove them.

I heard at one point, that sites running any Flash will be actively blocked by Chrome. If that is the case, that would be a huge problem.

If we don't use the 'chart', 'piechart', 'cartesianchart' and the derived xtypes, we are safe. The code of those components could be stripped away in ext-all-debug.js and be minified again in ext-all.js

Having two "deprecated" errors every time you use the Installer is just annoying, and embarrassing that it should still be an issue after four years. Is there someone, anyone, who can look at it and see if, at the least, the errors can be removed? Are they actually not working correctly? Are they actually going to be removed in MODX 3?

By the way, there is also a "Recent Documents" tab in "System Information" section, in addition to https://github.com/modxcms/revolution/issues/13199#issuecomment-673006509

Was this page helpful?
0 / 5 - 0 ratings