Fixes GH-201 - Assets do not move to "Active Assets" section when toggled "On" #202
Conversation
|
Hi @richard-to. Thank you for the PR! Active means that it's both enabled and current time lies within its start date and end date. See https://github.com/wireload/screenly-ose/blob/master/assets_helper.py#L29-L31 I think proper solution would be to calculate and set |
|
Thanks for the clarification. I'll update the pull request tomorrow. Thanks. |
|
Thanks @richard-to and @Kentzo. |
Add is_active, backup, rollback methods to Asset model
Conflicts: static/js/screenly-ose.coffee static/js/screenly-ose.js
|
Ok, I added the correct logic for active assets. I had forgotten to create a new branch originally, so I also have a few changes for GH-199 too. I moved the logic to the model instead. Also I updated the Jasmine unit tests so that they at least work. I added unit tests for this pull request and the other one. |
| at = now() | ||
| start_date = new Date(@get('start_date')); | ||
| end_date = new Date(@get('end_date')); | ||
| return start_date < at and end_date > at |
There was a problem hiding this comment.
Shouldn't this be an inclusive comparison, e.g. start_date <= at and end_date => at?
By the way, coffeescript supports Python like chained comparisons. So you can do this: start_date <= at <= end_date for simplicity.
|
The tests look nice. |
|
@aljungberg: For the dates, I agree that it makes sense for start and end to be inclusive. I was going by the backend code that @Kentzo linked here: https://github.com/wireload/screenly-ose/blob/master/assets_helper.py#L29-L31 |
|
Yes feels like the backend code should do the same. And it can also be updated to use a chained comparison for that matter. |
…ntend and backend logic
|
Ok, I updated the frontend/backend to be inclusive now. The chained comparison is definitely easier to read. |
Fixes GH-201 - Assets do not move to "Active Assets" section when toggled "On"
|
Awesome. Thanks @richard-to. |
The View code was checking the Asset model's
is_activeproperty. Unfortunately that property does not exist. The closest property isis_enabled. I usedis_enabledsince it looks like the backend expects that property.