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

New Feature: Publish start/end date #9

Closed
wants to merge 3 commits into from
Closed
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
9 changes: 8 additions & 1 deletion publisher/managers.py
Original file line number Diff line number Diff line change
@@ -1,4 +1,6 @@
from django.db import models
from django.db.models import Q
from django.utils import timezone

from .signals import publisher_pre_delete
from .middleware import get_draft_status
Expand All @@ -16,7 +18,12 @@ def drafts(self):

def published(self):
from .models import PublisherModelBase
return self.filter(publisher_is_draft=PublisherModelBase.STATE_PUBLISHED)

return self.filter(
Q(publication_start_date__isnull=True) | Q(publication_start_date__lte=timezone.now()),
Q(publication_end_date__isnull=True) | Q(publication_end_date__gt=timezone.now()),
publisher_is_draft=PublisherModelBase.STATE_PUBLISHED,
)
Copy link
Member

@suutari-ai suutari-ai Aug 31, 2017

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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?

Copy link
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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?

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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...)

Copy link
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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()

Copy link
Member

@suutari-ai suutari-ai Sep 2, 2017

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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.

Copy link
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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.


def current(self):
if get_draft_status():
Expand Down
35 changes: 35 additions & 0 deletions publisher/models.py
Original file line number Diff line number Diff line change
@@ -1,6 +1,8 @@
from django.utils import timezone
from django.db import models
from django.core.exceptions import ObjectDoesNotExist
from django.utils.translation import ugettext_lazy as _
from django.utils import timezone

from .managers import PublisherManager
from .utils import assert_draft
Expand Down Expand Up @@ -33,6 +35,23 @@ class PublisherModelBase(models.Model):

publisher_published_at = models.DateTimeField(null=True, editable=False)

publication_start_date = models.DateTimeField(
_("publication start date"),
null=True, blank=True, db_index=True,
help_text=_(
"Published content will only be visible from this point in time."
" Leave blank if always visible."
)
)
publication_end_date = models.DateTimeField(
_("publication end date"),
null=True, blank=True, db_index=True,
help_text=_(
"When to expire the published version."
" Leave empty to never expire."
),
)

publisher_fields = (
'publisher_linked',
'publisher_is_draft',
Expand Down Expand Up @@ -60,6 +79,22 @@ def is_draft(self):
def is_published(self):
return self.publisher_is_draft == self.STATE_PUBLISHED

@property
def hidden_by_end_date(self):
if not self.publication_end_date:
return False
return self.publication_end_date <= timezone.now()

@property
def hidden_by_start_date(self):
if not self.publication_start_date:
return False
return self.publication_start_date >= timezone.now()

@property
def is_visible(self):
return self.is_published and (not self.hidden_by_end_date) and (not self.hidden_by_start_date)

@property
def is_dirty(self):
if not self.is_draft:
Expand Down
115 changes: 113 additions & 2 deletions tests/myapp/tests.py
Original file line number Diff line number Diff line change
Expand Up @@ -2,7 +2,7 @@

from django import test
from django.utils import timezone

from django.utils.timezone import now as tz_now
from mock import MagicMock

from publisher.utils import NotDraftException
Expand All @@ -11,7 +11,6 @@

from myapp.models import PublisherTestModel


class PublisherTest(test.TestCase):

def test_creating_model_creates_only_one_record(self):
Expand Down Expand Up @@ -316,3 +315,115 @@ def test_middleware_forgets_current_draft_status_after_request(self):
PublisherMiddleware.process_response(None, None)

self.assertFalse(get_draft_status())

def test_publication_start_date(self):
yesterday = tz_now() - datetime.timedelta(days=1)
tomorrow = tz_now() + datetime.timedelta(days=1)

instance = PublisherTestModel.publisher_manager.create(title='Test model')
instance.publish()

# No publication_start_date set:

published = PublisherTestModel.publisher_manager.published()
self.assertEqual(published.count(), 1)
# Check model instance
obj = published[0]
self.assertEqual(obj.publication_start_date, None)
self.assertEqual(obj.publication_end_date, None)
self.assertEqual(obj.is_published, True)
self.assertEqual(obj.hidden_by_end_date, False)
self.assertEqual(obj.hidden_by_start_date, False)
self.assertEqual(obj.is_visible, True)

# Hidden, because publication_start_date is in the future:

instance.publication_start_date = tomorrow
instance.save()
instance.publish()
published = PublisherTestModel.publisher_manager.published()
self.assertEqual(published.count(), 0)
count = PublisherTestModel.publisher_manager.all().count()
self.assertEqual(count, 2) # draft + published
draft = PublisherTestModel.publisher_manager.drafts()[0]
self.assertEqual(draft.publication_start_date, tomorrow)
# Check model instance
obj = PublisherTestModel.objects.filter(publisher_is_draft=PublisherTestModel.STATE_PUBLISHED)[0]
self.assertEqual(obj.publication_start_date, tomorrow)
self.assertEqual(obj.publication_end_date, None)
self.assertEqual(obj.is_published, True)
self.assertEqual(obj.hidden_by_end_date, False)
self.assertEqual(obj.hidden_by_start_date, True)
self.assertEqual(obj.is_visible, False)

# Visible, because publication_start_date is in the past:

instance.publication_start_date = yesterday
instance.save()
instance.publish()
published = PublisherTestModel.publisher_manager.published()
self.assertEqual(published.count(), 1)
# Check model instance
obj = published[0]
self.assertEqual(obj.publication_start_date, yesterday)
self.assertEqual(obj.publication_end_date, None)
self.assertEqual(obj.is_published, True)
self.assertEqual(obj.hidden_by_end_date, False)
self.assertEqual(obj.hidden_by_start_date, False)
self.assertEqual(obj.is_visible, True)


def test_publication_end_date(self):
yesterday = tz_now() - datetime.timedelta(days=1)
tomorrow = tz_now() + datetime.timedelta(days=1)

instance = PublisherTestModel.publisher_manager.create(title='Test model')
instance.publish()

# No publication_end_date set:
published = PublisherTestModel.publisher_manager.published()
self.assertEqual(published.count(), 1)
# Check model instance
obj = published[0]
self.assertEqual(obj.publication_start_date, None)
self.assertEqual(obj.publication_end_date, None)
self.assertEqual(obj.is_published, True)
self.assertEqual(obj.hidden_by_end_date, False)
self.assertEqual(obj.hidden_by_start_date, False)
self.assertEqual(obj.is_visible, True)

# Hidden, because publication_end_date is in the past:
instance.publication_end_date = yesterday
instance.save()
instance.publish()
published = PublisherTestModel.publisher_manager.published()
self.assertEqual(published.count(), 0)
count = PublisherTestModel.publisher_manager.all().count()
self.assertEqual(count, 2) # draft + published
draft = PublisherTestModel.publisher_manager.drafts()[0]
self.assertEqual(draft.publication_start_date, None)
self.assertEqual(draft.publication_end_date, yesterday)
# Check model instance
obj = PublisherTestModel.objects.filter(publisher_is_draft=PublisherTestModel.STATE_PUBLISHED)[0]
self.assertEqual(obj.publication_start_date, None)
self.assertEqual(obj.publication_end_date, yesterday)
self.assertEqual(obj.is_published, True)
self.assertEqual(obj.hidden_by_end_date, True)
self.assertEqual(obj.hidden_by_start_date, False)
self.assertEqual(obj.is_visible, False)

# Visible, because publication_end_date is in the future:
instance.publication_end_date = tomorrow
instance.save()
instance.publish()
published = PublisherTestModel.publisher_manager.published()
self.assertEqual(published.count(), 1)
# Check model instance
obj = published[0]
self.assertEqual(obj.publication_start_date, None)
self.assertEqual(obj.publication_end_date, tomorrow)
self.assertEqual(obj.is_published, True)
self.assertEqual(obj.hidden_by_end_date, False)
self.assertEqual(obj.hidden_by_start_date, False)
self.assertEqual(obj.is_visible, True)