wilr / silverstripe-googlesitemaps

Google Sitemaps module for the SilverStripe CMS
BSD 3-Clause "New" or "Revised" License
74 stars 95 forks source link

FIX/NEW for #90 Allow ability to alter DataList and deprecate static interface for better extensibility. #91

Closed patricknelson closed 9 years ago

patricknelson commented 9 years ago

Fix (or adding new functionality) for #90

I also did some minor touch up of return documentation and return values. In this case, I just setup an empty string return at the end of get_frequency_for_class(). I'd do more but that would be much more out of scope for this issue/PR plus I don't have much time to look into that.

Anyway, here's an example for how you'd make use of this feature:

config.yaml

GoogleSitemap:
  extensions:
    - GoogleSitemapCustomExtension

mysite/code/GoogleSitemapCustomExtension.php

<?php

class GoogleSitemapCustomExtension extends Extension {
    /**
     * Ensure that only specific site domain ID's are allowed in GoogleSitemap module for front-end display. Note the use
     * of &$dataList being passed by reference. This is because while the class itself is still a reference (by default)
     * in PHP, we must actually modify the original variable that is referring to that variable. 
     * 
     * @param   DataList    $dataList
     * @param   string  $class
     */
    public function alterDataList(DataList &$dataList, $class) {
        if (strtolower($class) == "sitetree") {
            $dataList = $dataList->innerJoin("Page_Live", "Page_Live.ID=SiteTree_Live.ID");
            $dataList = $dataList->where("SiteDomainID=0 OR SiteDomainID=2");
        }
    }
}
patricknelson commented 9 years ago

FYI @wilr I'm noticing that I'm not calling alterDataList in all the appropriate spots. I'll move my...

$this->extend("alterDataList", $instances, $class);

... lower down so it gets called properly after the instances is set outside of that group of if statements. Sorry I missed that.

patricknelson commented 9 years ago

Ok -- put that into a separate commit. Give this a shot and let me know what you think!

https://github.com/patricknelson/silverstripe-googlesitemaps/commit/a641ded9263541cca8f662e76a942d6fa50d6afb

patricknelson commented 9 years ago

Bump @wilr did you get a chance to review yet? It shouldn't be a dramatic shift as the only major difference is the incorporation of new methods into the API. No official deprecation notices (i.e. via user_notice), just using old school @deprecated and etc.

wilr commented 9 years ago

All good. Thanks @patricknelson. Tests pass so happy for this to be merged.