Refactor sitemaps to use centralised events #5069
Conversation
| events.emit(model.resourceType() + '.published.edited', model); | ||
| } | ||
|
|
||
| events.emit(model.resourceType() + '.edited', model); |
This comment was marked as abuse.
This comment was marked as abuse.
Sorry, something went wrong.
This comment was marked as abuse.
This comment was marked as abuse.
Sorry, something went wrong.
| @@ -24,10 +25,16 @@ function BaseSiteMapGenerator() { | |||
| } | |||
|
|
|||
| _.extend(BaseSiteMapGenerator.prototype, { | |||
| dataEvents: events, | |||
This comment was marked as abuse.
This comment was marked as abuse.
Sorry, something went wrong.
|
@jgable thanks for the feedback, all good ideas! I'm currently a little bit stuck on how to write tests for this - once I've got the tests in I'll be able to be certain the logic transferred correctly - until then the answer is just 'I think so' haha. If anyone has any good links on patterns for testing event emitters send em my way - otherwise I think my problem is trying to rewrite the current integration tests when I probably need to change it completely into two sets - one to test the events get fired correctly on the models and one to test that when firing the events the sitemap behaves correctly. |
ErisDS
force-pushed
the
event-refactor
branch
2 times, most recently
from
12ed0b8 to
8d1eb83
Compare
ErisDS
changed the title
ErisDS
force-pushed
the
event-refactor
branch
2 times, most recently
from
08be1a9 to
b18094c
Compare
|
Have updated this with the recommended changes + tests on both the models and the sitemap. Also changed the way the user events were fired and picked up so that invited users are no longer included fixing #5104. |
fixes TryGhost#5104, refs TryGhost#4348, TryGhost#2263 - Create a centralised event module - Hook it up for posts, pages, tags and users - Use it in sitemaps instead of direct method calls - Use it for xmlrpc calls - Check events are fired in model tests - Update sitemap tests to work with new code - Fix a bug where invited users were appearing in sitemaps - Move sitemaps and xmlrpc into a directory together
|
I've updated this one again with two additional changes:
|
|
Sorry, haven't had time to really give this a thorough look through. But, it looks good to me from a cursory glance. Doesn't seem like it could hurt with all the unit tests working and fixes for xmlrpc.
|
Refactor sitemaps to use centralised events
This is the result of me being bored on a plane for a few hours, it's a refactor of the sitemap generation to use a centralised event system.
The idea is to use this for sitemaps and XMLRPC, potentially RSS and other things too.
This is now a working PR, with tests and a bugfix. Once this is in, I'll do further work to update XMLRPC and RSS to be generated based on these events.
fixes #5104, refs #4348, #2263