Questions relating to old extension hooks. about silverstripe-googlesitemaps HOT 5 CLOSED

@patricknelson - did you make any progress with this? I'd love to see some documentation/ examples around the extension hooks.

from silverstripe-googlesitemaps.

It depends on what you mean by "progress" @TamaEaston 😄 Sure, I made progress in that I solved my specific use case issues, for example I have a central ->alterDataList(...) hook where I ensure that you don't even list page types which have zero results for the current domain (since I have a custom implementation of a multi-domain website).

To clarify (now that you caused me to think about it a bit more) I think the core point to opening this issue was more to discuss if it made sense to:

1. Eliminate just specifically updateGoogleSitemapItems, since now we have a new alterDataList hook (which directly handles exactly the same task but more centrally and less repetitively). I think it makes sense to retain the other hook (updateGoogleSitemapItems) as it still provides utility that is not precisely solved by just having alterDataList since those are two different lists with different contents.
2. Migrate the other hook updateGoogleSitemaps to being triggered on the root GoogleSitemap class to help simplify things. Currently you (unfortunately) have to setup yet another extension just to affect the sitemaps listed in the controller even though it is entirely sourced from GoogleSitemap::inst()->getSitemaps() -- why extend a controller to alter that list when it's sourced there? I understand it's only currently used in the controller, but extending two different objects when it otherwise would be easy to simply extend GoogleSitemaps makes no sense.
3. Possibly refactor GoogleSitemapExtension to GoogleSitemapPageExtension to better define it's use/purpose because if you do want to extend GoogleSitemap you cannot use the most intuitive name as it's already taken by the aforementioned page extension. Confusing, I know...

For example, if you utilized all of the extension hooks (like I currently do):

/**
 * IMPORTANT: Class name contains superfluous "Custom" word because "GoogleSitemapExtension" is
 * already setup in the module itself (see #3 above).
 */
class GoogleSitemapCustomExtension extends Extension {

    /**
     * @param   DataList    $dataList
     * @param   string  $class
     */
    public function alterDataList(DataList &$dataList, $class) {
        // alter $dataList as you see fit...
    }

}

class GoogleSitemapControllerExtension extends Extension {

    /**
     * @param ArrayList $sitemaps
     */
    public function updateGoogleSitemaps(ArrayList &$sitemaps) {
        // ... modify the root sitemaps that can be output to the front-end.
    }

    /**
     * @param   ArrayList   $items
     * @param   string      $class
     * @param   int         $page
     */
    public function updateGoogleSitemapItems($items, $class, $page) {
        // ... this doesn't make sense anymore due to alterDataList on the
        // aptly named GoogleSitemapCustomExtension above...
    }

}

from silverstripe-googlesitemaps.

@patricknelson - that is awesomeness above and beyond the call of duty. Thank you.

from silverstripe-googlesitemaps.

I like to be thorough!

from silverstripe-googlesitemaps.

Patches welcome!

from silverstripe-googlesitemaps.