Load feeds on dashboard with AJAX instead of on page render #13507
Conversation
…delays from slowing down the manager
|
|
||
| $enabled = $this->modx->getOption('feed_modx_' . $feed . '_enabled', null, true); | ||
| if (!$enabled) { | ||
| return $this->failure(); |
There was a problem hiding this comment.
Is this really a failure ? Seems perfectly legit to have those disabled.
Or maybe is that because the request should have never occurred ?
There was a problem hiding this comment.
Yeah... i should have reviewed the whole diff first!
There was a problem hiding this comment.
Yeah, this shouldn't actually trigger. Was tempted to take it out, and happy to do so if you think that's better ;)
There was a problem hiding this comment.
No, i'm fine with it (we can never assume only core components would use it).
I was just concerned about the "failure" semantic...
|
|
||
| public function loadFeed($url) | ||
| { | ||
| $this->modx->loadClass('xmlrss.modRSSParser','',false,true); |
There was a problem hiding this comment.
No particular reason. Mostly just copied the old code from the widgets into the processor, and that used it like this... getService is probably cleaner though.
|
Updated it to use getService instead :) |
…g AJAX [#13507] * upstream/pr/13507: Use $modx->getService instead of loadClass and direct instantiation of the RSS parser Remove sleep() call from testing the loading spinner Load the dashboard feeds (news/security) over AJAX to prevent remote delays from slowing down the manager
|
Working nicely, many thanks! |
|
#modxbughunt #1point to @rtripault #modxbughunt #3points to @Mark-H |
|
@Mark-H This might raise potential privacy issues as described here: Sending the requests via Ajax includes data like the referrer, which can be used to identify the MODX (manager) location. |
|
The AJAX request is only local to MODX; it still loads the feed server-side (just in a separate process) just like before. |
|
Ah gotcha! That of course if perfect! 👍 |
What does it do?
This changes the way the news and security feeds on the dashboard are rendered. Previously, this happened during rendering of the manager (i.e. before the manager HTML is sent to the browser). Now, it happens asynchronously when the manager page has been rendered by ExtJS.
Why is it needed?
The RSS feeds can be quite slow, especially in remote places. They also depend on an internet connection to work, which isn't always available. When the feeds are slow to load, the entire dashboard is slow to load, which is bad.
There are also other issues with the current RSS reader implementation, for example, #13209 where the entire manager is blank if the underlying RSS class can't find OpenSSL. By moving it into a separate PHP process, such issues will no longer break the entire manager just like that. I've not changed how that implementation works for now, other than putting the calls into a different place so it's more resilient.
Related issue(s)/PR(s)
I can't find a specific issue requesting this to be loaded this over AJAX, but #13209 and #2797 are somewhat related.