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
Always show CTA banner if no reading goal exists #8606
Always show CTA banner if no reading goal exists #8606
Conversation
$ year = current_year() | ||
$ current_goal = get_reading_goals(year=year) | ||
$ next_year = year + 1 | ||
$ next_goal = get_reading_goals(year=next_year) | ||
|
||
$# Is date between 1 Dec and 1 Feb? | ||
$if not current_goal and within_date_range(12, 1, 2, 1): | ||
$if (within_date_range(1, 1, 11, 30) and not current_goal) or (within_date_range(12, 1, 12, 31) and not next_goal): |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
$ year = current_year()
$ goal = get_reading_goals(year=year)
$if not goal:
...
rm within_date_range
Let's reapply on a fresh |
2683d51
to
7a45bd3
Compare
Noting here your observation about yearly reading goal "hide banner" cookie not working well on shared computers, @mekarpeles. |
$ cookie_name = 'yrg' | ||
$:render_template('site/banner', announcement, cookie_name, cookie_duration_days=65) | ||
$ cookie_name = 'yrg%d' % (year % 100) | ||
$:render_template('site/banner', announcement, cookie_name, cookie_duration_days=365) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Instead of 365, we should use the number of days until the end of the year.
Note: either way, this shouldn't impact site behavior because the cookie name is pinned to the year and so one year cannot bleed and effect another year.
Closes #8580
Ensures that the main My Books page always shows one of the following:
Technical
The
get_reading_goals_year
function has been removed, as its sole purpose was to aid in showing the banner during December.within_date_range
and its unit tests have also been removed.Banner cookie name was changed to
yrgYY
, whereYY
is the last two digits of the current year. Cookie duration has been increased to one year.Important: After deployment, any patron who has hidden this banner will have to hide the banner one more time (due to the cookie change).
Testing
Screenshot
Stakeholders
@mekarpeles