-
Notifications
You must be signed in to change notification settings - Fork 84
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
As a user I want to have the Coming Up field active when I create a new story. [SDBELGA-466] #2298
Conversation
…ew story. [SDBELGA-466]
imo it's not exactly clear what this does from template perspective:
might be better something like:
|
Doesn't this will mean that we are applying two filters?
|
yep that would be 2 filters |
apps/templates/filters.py
Outdated
@@ -63,10 +63,15 @@ def first_paragraph_filter(input_string): | |||
return "" | |||
|
|||
|
|||
def iso_datetime(date, initial_offset): | |||
def add_timedelta(date, minutes): |
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.
it could be with **kwargs
so it's not limited to use minutes
apps/templates/filters.py
Outdated
try: | ||
return date + timedelta(minutes=minutes) | ||
return date + timedelta(minutes=kwargs.get("minutes")) |
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.
it should be rather timedelta(**kwargs)
here so there can be hours/seconds/whatever
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.
yep, I will do the changes.
features/templates.feature
Outdated
@@ -736,7 +736,7 @@ Feature: Templates | |||
"test": "{{ user.sign_off }}", | |||
"test2": "{{ item.something.missing }}", | |||
"test3": "{% if something %}", | |||
"test4": "{{ now|iso_datetime }}" | |||
"test4": "{{ now|add_timedelta(minutes=45)|iso_datetime }}" |
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.
what/how does it actually test it?
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.
I think we don't require it now before that test case is failing here on first commit, and later you suggested creating two filters so thought we should add another filter also there. is it ok if I remove that add_timedelta filter from here?
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.
I think it would be good to test the filter, I'm just not sure what are we testing here exactly
…ew story. [SDBELGA-466] (#2298) * As a user I want to have the Coming Up field active when I create a new story. [SDBELGA-466] * Address comments * minor change * use kwargs in timedelta * Add test case * minor change
No description provided.