Update schedule with timezone information #902
Conversation
…ding to except, tests, make dashboard edit except date have time zone information Signed-off-by: Aaron Chong <[email protected]>
Codecov ReportAttention:
Additional details and impacted files@@ Coverage Diff @@
## deploy/hammer #902 +/- ##
=================================================
- Coverage 49.35% 46.43% -2.93%
=================================================
Files 285 204 -81
Lines 7564 6319 -1245
Branches 1050 820 -230
=================================================
- Hits 3733 2934 -799
+ Misses 3682 3379 -303
+ Partials 149 6 -143
Flags with carried forward coverage won't be shown. Click here to find out more. ☔ View full report in Codecov by Sentry. |
| @@ -1,3 +1,4 @@ | |||
| # pylint: disable=too-many-lines | |||
There was a problem hiding this comment.
Don't disable the linter without a valid reason. I think the line count is probably due to duplicated code, see if it is possible to refactor them. If that is still an issue, then disable the rule globally (might want to do it regardless, imo this rule is not useful at all).
There was a problem hiding this comment.
Hmm, but this is only for testing though. We could refactor it and re-used code between tests, but I was trying to prevent any stack tracing, even if the checks are repeated over tests, it doesn't make much sense to have the errors on the same lines across tests.
There was a problem hiding this comment.
I think it is really up for debate whether code duplication is justified to make test more clear. I don't really have any strong opinions, if you think that it is ok, then just disable the lint globally. imo that is not a useful rule anyway, regardless of the reasons.
| @@ -234,7 +239,8 @@ async def update_schedule_task( | |||
|
|
|||
| async with tortoise.transactions.in_transaction(): | |||
| if except_date: | |||
| event_date_str = except_date.isoformat() | |||
| event_date_str = convert_date_server_timezone_iso_str(except_date) | |||
| logger.info(f"Updating event with iso format date: {event_date_str}") | |||
There was a problem hiding this comment.
Include the subject (task id).
There was a problem hiding this comment.
Thanks for catching that, that'll be useful. I've also added more logs in the route function
66db893
We can move logs to debug during the next phase of the project when these features stabilize
Signed-off-by: Aaron Chong <[email protected]>
Signed-off-by: Aaron Chong <[email protected]>
|
Overall the update route is better off refactored into 2 routes, one for updating an entire schedule, the other for updating just an event. I've noted it down as a next step in our project, but for now this PR is a required fix without breaking API |
What's new
Similar to #896, but for schedule editing
Self-checks