Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Page method: `->isPublished()` #375

Closed
afbora opened this issue Aug 10, 2019 · 12 comments
Closed

Page method: `->isPublished()` #375

afbora opened this issue Aug 10, 2019 · 12 comments
Assignees
Labels
Projects

Comments

@afbora
Copy link

@afbora afbora commented Aug 10, 2019

Check page is published like that:

if(!$page->isDraft()) {
   // do stuff
}

or

if($page->isListed() or $page->isUnlisted()) {
   // do stuff
}

It would be nice to have a method in Kirby core like this:

if($page->isPublished()) {
   // do stuff
}
@PaulMorel

This comment has been minimized.

Copy link

@PaulMorel PaulMorel commented Aug 12, 2019

Wouldn't it be exactly the same as isListed() ?

@afbora

This comment has been minimized.

Copy link
Author

@afbora afbora commented Aug 12, 2019

@PaulMorel No. Listed + Unlisted = Published

@texnixe

This comment has been minimized.

Copy link

@texnixe texnixe commented Aug 12, 2019

You can create a custom page method or use

if (page->isDraft() === false) {
  // do stuff
}

as you already suggested above. Do we really need an additional method?

@afbora

This comment has been minimized.

Copy link
Author

@afbora afbora commented Aug 12, 2019

@texnixe Already i use with !isDraft(). I think it should be in the core because there is such a page status as published.

And same useful methods such as: isEmpty() and isNotEmpty()

@foobartel

This comment has been minimized.

Copy link

@foobartel foobartel commented Aug 13, 2019

No. Listed + Unlisted = Published

IMHO unlisted != published.

@texnixe

This comment has been minimized.

Copy link

@texnixe texnixe commented Aug 13, 2019

@foobartel Both listed and unlisted pages are published pages, i.e. they can be accessed by anyone using the page Url. Only draft pages are inaccessible to the public.

https://github.com/getkirby/kirby/blob/03d6e96aa27f631e5311cb6c2109e1510505cab7/src/Cms/Pages.php#L446

@foobartel

This comment has been minimized.

Copy link

@foobartel foobartel commented Aug 13, 2019

@texnixe Technically that's absolutely correct ;) I guess the way I mostly use unlisted changes how I look at it and don't view pages as (fully) "published".

@texnixe

This comment has been minimized.

Copy link

@texnixe texnixe commented Aug 13, 2019

@foobartel Yes, but here we have to look at how published is defined, not how you or I personally use it. I use unlisted pages for multiple purposes, e.g. those that should not appear in the main menu but a secondary menu only, for different state in a publishing workflow etc.

@texnixe

This comment has been minimized.

Copy link

@texnixe texnixe commented Aug 13, 2019

@afbora What is your use case for this anyway? $pages always refers to published pages anyway, unless you specifically call on childrenAndDrafts(). I'm very much in favor of keeping the standard methods to what is really needed as more methods that are only needed for edge cases will only confuse or overwhelm people. Just my 2ct.

@foobartel

This comment has been minimized.

Copy link

@foobartel foobartel commented Aug 13, 2019

@texnixe Apologies if I haven't been clear enough in my last message, but I absolutely do agree with your feedback and hence #375 (comment) as well as the implementation ;)

@afbora

This comment has been minimized.

Copy link
Author

@afbora afbora commented Aug 13, 2019

@texnixe I need this method in hooks like that:

return [
    'hooks' => [
        'page.update:after' => function ($newPage, $oldPage) {
            if(!$newPage->isDraft()) {
               // do stuff
            }
        }
    ]
]

Yes, it may not be used very often, but a page may be listed or unlisted, and I don't think the method that check both of these would be redundant.

@distantnative distantnative changed the title New page method: isPublished() Page method: `->isPublished()` Mar 5, 2020
@distantnative distantnative added this to Backlog in Roadmap via automation Mar 5, 2020
@distantnative distantnative moved this from Backlog to Low-hanging fruits in Roadmap Mar 5, 2020
@distantnative distantnative self-assigned this Mar 5, 2020
distantnative added a commit to getkirby/kirby that referenced this issue Mar 5, 2020
@distantnative distantnative mentioned this issue Mar 5, 2020
4 of 4 tasks complete
bastianallgeier added a commit to getkirby/kirby that referenced this issue Mar 9, 2020
@bastianallgeier

This comment has been minimized.

Copy link
Contributor

@bastianallgeier bastianallgeier commented Mar 9, 2020

Roadmap automation moved this from Low-hanging fruits to Done Mar 9, 2020
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
Roadmap
  
Done
Linked pull requests

Successfully merging a pull request may close this issue.

None yet
6 participants
You can’t perform that action at this time.