Issue 686 - Use timestamp instead of string of timestamp to represent image modified date #689
Conversation
|
did you contacted the framework team about this issue? |
rodfersou
changed the title
| @@ -158,10 +159,12 @@ def modified(self): | |||
| name='images', | |||
| default=None) | |||
| return base_scales and base_scales.modified() | |||
| mtime = '' | |||
| mtime = DateTime(0) | |||
There was a problem hiding this comment.
this should start as None; check the related code in plone.scale.
| self.scaling = ImageScaling(self.tile, self.request) | ||
|
|
||
| def test_modified(self): | ||
| self.assertIsInstance(self.scaling.modified(), DateTime) |
There was a problem hiding this comment.
this test is useless; we need to deal with scales
rodfersou
force-pushed
the
issue_686
branch
2 times, most recently
from
8ae21ca
to
829254a
Compare
rodfersou
changed the title
rodfersou
force-pushed
the
issue_686
branch
2 times, most recently
from
9a1f8e0
to
4b278f7
Compare
| for k, v in self.context.data.items(): | ||
| if INamedImage.providedBy(v): | ||
| mtime += self.context.data.get('{0}_mtime'.format(k), '') | ||
| image_time = self.context.data.get('{0}_mtime'.format(k), None) | ||
| if mtime is None or image_time > mtime: |
There was a problem hiding this comment.
@rodfersou can you explain this, please? are there any tiles with more than one image field?
There was a problem hiding this comment.
The code at tiles/data.py is made to search any image field and set the modified date into a data attribute.
I think this is done this way because there is no way to know if someone creates a tile with more than one image.
On the other hand, the timestamp setted at the same code will be the same for all image items.. so we could cut the complexity and create just one attribute for every image, removing the need to make a loop and check if the item is INamedImage.
I don't know if a refactory at this code is beyond the borders of this pull request, or if this is a good time to do it.
There was a problem hiding this comment.
AFAIK, there are no tiles with more than one image field, so let's not solve problems we don't have; anyway all that code is overly complex and has to be completely refactored at some point.
hvelarde
force-pushed
the
issue_686
branch
2 times, most recently
from
84a6411
to
7ceede2
Compare
closes #686