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

Wagtail - Banners: Make Data inactive (expiration) date field optional #4300

Closed
1 task
Tracked by #138
dorothyyeager opened this issue Jan 4, 2021 · 7 comments
Closed
1 task
Tracked by #138
Assignees

Comments

@dorothyyeager
Copy link
Contributor

Summary

What we are after:
For long-term banners, it's easy to forget when they expire. It would be better if we didn't have to fill out that field at all so we don't have a situation where a banner expires during a time when people are out (for example, the December holiday season).

Completion criteria

  • Date inactive field is made optional (see screenshot)

image

@dorothyyeager dorothyyeager added this to the Sprint 13.12 milestone Jan 4, 2021
@pkfec pkfec changed the title Wagtail - Banners: Make expiration date field optional Wagtail - Banners: Make Data inactive (expiration) date field optional Jan 7, 2021
@pkfec pkfec modified the milestones: Sprint 13.12, PI 13 innovation 2 and PI 14 planning Jan 26, 2021
@johnnyporkchops
Copy link
Contributor

johnnyporkchops commented Jan 27, 2021

@dorothyyeager If you want a banner to never expire, you can always just give it a date far in the future like 2035. My only concern with also having the option to leave it blank is the more we keep updating the way the feature works, the more confusing it can get for both editors and developers. Would be interested in your thoughts on this (@dorothyyeager)

For example, when we added the "Active" checkbox, one could already make a banner active/inactive by just publishing/unpublishing it and that checkbox has occasionally led to some confusion.

If we add more logic to make a banner persist indefinitely if the date inactive field were left blank, we would also want to add more help-text to let editors know that is an option. Like the "Active" checkbox we would be adding another option for something that already exists. In addition to it possibly becoming more confusing for editors, the underlying logic seems unnecessarily complicated :

alert_banners = AlertForEmergencyUseOnly.objects.live().filter((Q(
        alert_active=True, alert_date_active__lte=datetime_now,
        alert_date_inactive__gt=datetime_now) | (Q(alert_active=True,alert_date_inactive__isnull=True)))).order_by('-alert_date_active')[:2]

@dorothyyeager
Copy link
Contributor Author

Talked with @johnnyporkchops and content team just now. Because he's raised valid concerns about making the logic confusing, we'll go with the proposed workaround if @AmyKort @patphongs are OK with it (give us a thumbs up here).

Workaround for things like government shutdowns and COVID banners where we don't know end date: We'll set the "date inactive" field to ten years in the future. I'll document this in the "how to do banners" spreadsheet and instructions.

Reassigning ticket to me to get sign off and update the instructions. Thanks @johnnyporkchops for looking into this!

@johnnyporkchops
Copy link
Contributor

Thanks @dorothyyeager. Maybe in an innovation sprint, I would not mind working with content team to evaluate the home page banner interface so that we can rename the fields in a way that makes more sense to editors and also add the options that work better for the way you actually use the feature. This might also include extending Wagtail to have more prominent and readable help-text.

@johnnyporkchops
Copy link
Contributor

There is also this ticket regarding active/inactive dates that makes me think we should evaluate the functionality from scratch and see how we can accommodate the way content team schedules home banners
#4022

@dorothyyeager
Copy link
Contributor Author

I'll make sure these tickets are in our Content epic planning doc-might be a good sub-epic for the Content team and you to pair on in future.

@johnnyporkchops
Copy link
Contributor

That sounds great @dorothyyeager , thanks!

@dorothyyeager
Copy link
Contributor Author

Closed. We'll do our workaround and that's been added to the spreadsheet and instructions for banners.

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

No branches or pull requests

3 participants