Skip to content
This repository has been archived by the owner on Sep 16, 2021. It is now read-only.

Move publish workflow checker to provider decorator #221

Closed
wouterj opened this issue Feb 17, 2015 · 13 comments · Fixed by #225
Closed

Move publish workflow checker to provider decorator #221

wouterj opened this issue Feb 17, 2015 · 13 comments · Fixed by #225

Comments

@wouterj
Copy link
Member

wouterj commented Feb 17, 2015

We should have a VotingProvider which decorates a UrlInformationProviderInterface implementation. This is then used to vote if the UrlInformation is shown. The publish workflow checker would then be one voter.

People might want to add more features, like ACL checks (as used by the kunstmaan guys Kunstmaan/KunstmaanBundlesCMS#46 ).

This also means the UrlInformation class should have a new property: $object which contains the document/entity.

@wouterj
Copy link
Member Author

wouterj commented Feb 17, 2015

@dbu / @lsmith77 looking at the issues and PRs today about the Sitemap feature, I don't think we can call this bundle/feature stable and ready to release a new version soon.

@ElectricMaxxx
Copy link
Member

Is there a chance for a little training in Naming of different Services and its purpose? You mentioned Voters, enhancers, Guesser and Providers in these two issues. Together with the decoration and a Guinness my head can't really follow you.
But in general i understand to implement a mechanism if an document is exposed to the sitemap and one to extract the values, right?

@ElectricMaxxx
Copy link
Member

Btw: it will be necessarry to cache that result like We do it for seo metadata after extraction.

@wouterj
Copy link
Member Author

wouterj commented Feb 17, 2015

Voters
Voters are classes that vote for something. E.g. Security votes get a User and they all vote "deny access", "allow access", "cannot handle this" (the publish workflow checker works also with voters). Another example of a voter is used in the KnpMenuBundle, to vote if a menu item is the current page or not.

Enhancers
These can edit objects/values after they are created.

Guessers
These are guessing the value of an option. They are like dynamic defaults. An example of a guesser is used by the Form component: when no type is defined for a field, it tries to guess the type based on doctrine mapping information and PHPdoc annotations.

Providers
Providers provide some values. E.g. the UrlInformationProvider provides a list of urls and their information.

Decoration
This is a design pattern that takes an instance and decorates around it (before or after). Like:

class VoterProvider implements UrlInformationProviderInterface
{
    private $provider;
    private $voters;

    public function __construct(UrlInformationProviderInterface $provider, array $voters = array())
    {
        $this->provider = $provider;
        $this->voters = $voters;
    }

    public function getUrlInformations()
    {
        $urlInformations = $this->provider->getUrlInformations();

        foreach ($urlInformations as $id => $urlInformation) {
            foreach ($this->voters as $voter) {
                if (false === $voter->accept($urlInformation)) {
                    unset($urlInformations[$id]);
                    break;
                }
            }
        }

        return $urlInformations;
    }
}

@ElectricMaxxx
Copy link
Member

@wouterj that is awesome thanks. You should write a Book about that and Code Style.

Just for a furrher ordering in my mind:
I would let the enhancers/guesser work on the result set of the voters, cause that is a subset of all documents, right. So shouldn't we transform the route/document/object information into the UrlInformation in the enhancers/guessers and let the voters return a list of documents/object only with its routes?
And I think We should register both, guesser/enhancers and voters by tags right?

@ElectricMaxxx
Copy link
Member

Imagine we should provide a default List of enhancers and/or guessers as We do it for the seo metadata ectractors, which would you expect to be implemented?

I just have the title -> label by getTitle() in my mind, this would be a guesser, right. But how would you expect one for changeFrequency or the other properties? Which one would you expect in general? Should We implement both? Enhancers and guessers? In in priotizised list?

@ElectricMaxxx
Copy link
Member

And should'nt we start to use named sitemaps in the tag definition of voters/enhancers/guessers? So We would be able to produce different sitemaps based on different voters? You would be able to create a separate xml sitemap for google and one in HTML for the Users. The last one can be separated into subsets by an ACL mechanism translated into the voters. (@wouterj mentioned it at the beginning) that would be ultra hot.

@ElectricMaxxx
Copy link
Member

Slowly i understand why @wouterj is calling it a bundle ...
It is completely decoupled from the SeoStuff and We would be able to extract it into a new bundle. So We can tag the changes in Seo as stable.
If you would agree i would be happy if somebody created a CmfSitemapBundle with some rights for me or give me the rights to do so.

@dbu
Copy link
Member

dbu commented Feb 18, 2015

Was going to write that you can hook anything into the publish workflow. But then, decisions for the sitemap visibility are not the same as whether something is published or not. So i am +1 for having voters. not sure if we need a decorator or could just build it into the provider. performance wise its about the same, with the overhead of an extra class.

@dbu
Copy link
Member

dbu commented Feb 18, 2015

And when we do that +1 on having the sitemap "name" somehow available to give the voters a context. hm, or go with the decorator and have one service per sitemap context. i guess the voters knowing about a context name is not very elegant. configuring the voters for a specific context sounds more generic.

@ElectricMaxxx
Copy link
Member

@dbu you should also have a look at #222 please.

@dbu
Copy link
Member

dbu commented Feb 18, 2015

I guess another voter could be used to filter non-canonical urls for the same content - see also #218

i guess it depends what we generate - if we go over the /cms/content, there will be one url per document (in the requested language), but if we search /cms/routes, we would get all alternatives.

@dbu
Copy link
Member

dbu commented Feb 18, 2015

btw, i wonder if we should call the data for UrlInformation $object or rather $data or something. it could be an array with route name and parameters in the case of core symfony routing information.

Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Projects
None yet
Development

Successfully merging a pull request may close this issue.

3 participants