New Feature: Publish start/end date

[jedie]

See also: #7

[suutari-ai]

Thanks for the new feature, @jedie!

This is missing migrations though. Can you add the migration file and rebase this on master please?

[JsseL]

@jedie I'm also thinking that, is this actually a bit problematic. After this we would have objects in "draft" state, but actually public because of the dates.

Edit: Didn't read the PR carefully enough. Indeed looks like you made sure that publisher_is_draft was the primary field to check and dates had an effect only if it was considered public according to publisher_is_draft. 👍

[JsseL]

The few merge commits in the PR seem a bit unnecessary, otherwise looks pretty good to me.

[jedie]

I have rebase on master and add tests to this feature...

[jedie]

There's a silly thing about it.

If an instance has already been published and you then set an end date, you should not forget to publish these changes.

On the other hand, this is only consistent.

[jedie]

I'm also working on a more verbose, but compact admin column. Something like this:

    def visibility(self, obj):
        is_dirty = obj.is_dirty
        if obj.publisher_linked:
            obj = obj.publisher_linked

        if obj.is_visible:
            if is_dirty:
                text = _("Changed!")
                title = _("Old version is public, but has not published changes.")
            else:
                text = _("public")
                title = _("Is public.")
        elif not obj.is_published:
            text = _("not public")
            title = _("Not published, yet")
        else:
            text = _("hidden")
            if obj.hidden_by_end_date and obj.hidden_by_start_date:
                title = _("Published, but hidden by start/end date.")
            elif obj.hidden_by_start_date:
                title = _("Published, but hidden by start date.")
            elif obj.hidden_by_end_date:
                title = _("Published, but hidden by end date.")
            else:
                title = _("Published, but hidden.")

        return format_html(
            '<span title="{}">{}</span>', title, text
        )

    visibility.short_description = _("Visibility")
    visibility.allow_tags = True

maybe it can be similar to django cms icons in page list?!?

[suutari-ai]

Nice work, @jedie! Thanks!

Btw. forget my comment about missing migrations. Forgot that the changed models are abstract, so no migrations are needed.

[suutari-ai]

Hmm.. Should the functionality of this manager method match with the is_published method in PublisherModelBase? I think it should... so maybe add a visible method rather than changing this one?

[jedie]

Yes and no... Don't know ;)

The idea was not to change the current API. Existing software that uses published() also considers start/end date.

But yes, it's a little messy. What should I do?

[suutari-ai]

Yeah, I see. Maybe the is_published property should follow the same logic then? If there is a need for another property which doesn't consider the date, it should have a different name. How about is_publishable? (Yep, naming is hard...)

[jedie]

Yes, naming... Hm...

Maybe we should just risk to ignore start/end date and "refactor" the names a little bit:

instance bool properties:

• instance.is_draft
• instance.is_published
• instance.hidden_by_end_date
• instance.hidden_by_start_date
• instance.is_visible
• instance.is_dirty

Model Manager methods:

• PublisherManager.drafts()
• PublisherManager.published()
• PublisherManager.visible()

QuerySet methods:

• PublisherQuerySet.filter_drafts()
• PublisherQuerySet.filter_published()
• PublisherQuerySet.filter_visible()

It should be documented that user should switch from PublisherManager.published() to PublisherManager.visible()

[suutari-ai]

I think that is a good call.

Just a one thing: The QuerySet methods should have same names as the Model Manager methods. See https://docs.djangoproject.com/en/1.8/topics/db/managers/#from-queryset for instructions on how to define the drafts, published and visible in the query set so that they are also defined in the manager.

[jedie]

I have done this in #14 ... It is time-consuming to maintain all changes in separate pull requests. So I created a new pull request with all changes.