implement voter for SitemapAwareDocuments #247
Conversation
ElectricMaxxx
commented
Jul 30, 2015
ElectricMaxxx
commented
| Q | A |
|---|---|
| Bug fix? | no |
| New feature? | yes |
| BC breaks? | no |
| Deprecations? | no |
| Tests pass? | yes |
| Fixed tickets | #240 |
| License | MIT |
| Doc PR | symfony-cmf/symfony-cmf-docs#692 |
| */ | ||
| class SitemapAwareDocumentVoter implements VoterInterface | ||
| { | ||
|
|
ElectricMaxxx
force-pushed
the
feature/ISSUE-240_voter_for_sitemap_aware_docuemtents
branch
from
aa06325 to
3abdaa5
Compare
| */ | ||
| public function exposeOnSitemap($content, $sitemap) | ||
| { | ||
| return $content instanceof SitemapAwareInterface && $content->isVisibleInSitemap(); |
There was a problem hiding this comment.
the voters need to be unanimous, right? and there is no 3rd option "abstain"? i think whe need to say if (!$content instanceof SitemapAwareInterface) {return true;}
There was a problem hiding this comment.
if we don't split that, every content that does not implement the interface will automatically be hidden from all sitemaps. imo if we want only such models, it would be the job of the provider to not load others in the first place.
There was a problem hiding this comment.
i think, we just have a true/false return on the voters, right?
There was a problem hiding this comment.
ahhh now i understand.
|
lets squash the commits and merge, looks good! |
|
no, forgot to change my tests that fits the preor case |
change method signature fix cs and result fix tests fix ocurances on method call, fix tests
ElectricMaxxx
force-pushed
the
feature/ISSUE-240_voter_for_sitemap_aware_docuemtents
branch
from
c8fec78 to
52a1356
Compare
|
merge this when happy. |
|
this also needs to be documented |
|
Wrote symfony-cmf/symfony-cmf-docs#692 for that.
|
…sitemap_aware_docuemtents implement voter for SitemapAwareDocuments
ElectricMaxxx
deleted the
feature/ISSUE-240_voter_for_sitemap_aware_docuemtents
branch
