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

Purge configuration when delete tile #765

Closed
rodfersou opened this Issue Nov 9, 2017 · 5 comments

Comments

Projects
None yet
2 participants
@rodfersou
Member

rodfersou commented Nov 9, 2017

Steps to reproduce:

  • Add new tile
  • Go to compose tab
  • Edit configuration opening Edit URL in new tab (and keep this URL saved)
  • Delete tile
  • Visit again the URL, and the data is still there
@hvelarde

This comment has been minimized.

Member

hvelarde commented Nov 9, 2017

this is a critical issue we must fix ASAP; an upgrade step to clean all existing cover objects will be needed.

@rodfersou

This comment has been minimized.

Member

rodfersou commented Nov 15, 2017

Well, we already handle annotation deletion in this view that is called by this ajax request, but it looks like we have a big mess in the javascript event registration:

image

There are many events registered in the close button, and probably one of them has the instruction e.preventDefault() what makes the next other events don't happen. In other words, the deletetile view is never called.

By the way I also noticed that the Delete view should be in layout.py, not compose.py file.

@hvelarde

This comment has been minimized.

Member

hvelarde commented Nov 16, 2017

I can confirm this is an issue and that the delete() method is never reached when clicking the delete icon on the layout screen.

this is what I did:

  • create a cover with a layout of multiple rows and columns
  • add some tiles to the layout
  • configure the tiles pressing the Save button
  • delete the tiles and repeat the operation a couple of times

for every tile with a configuration saved an annotation is created and these are never deleted; after some tests I can see the following stored in the annotations even as no tile was left in the layout:

(Pdb) self.annotations
{'plone.tiles.configuration.c218d0eb-e896-4465-898a-a25cca9e1599': {'description': {'order': u'1', 'visibility': u'on'}, 'title': {'htmltag': u'h2', 'order': u'0', 'visibility': u'on'}, 'css_class': u'tile-default', 'image': {'position': u'left', 'imgsize': u'mini 200:200', 'visibility': u'on', 'order': u'2'}, 'subjects': {'order': u'4', 'visibility': u'on'}, 'date': {'order': u'3', 'visibility': u'on', 'format': u'datetime'}}, 'plone.tiles.configuration.e88dcdd1-3132-4044-9757-9079dc35ce75': {'count': {'order': u'2', 'visibility': u'on', 'offset': u''}, 'description': {'order': u'4', 'visibility': u'on'}, 'title': {'visibility': u'on', 'order': u'3', 'htmltag': u'h2'}, 'css_class': u'tile-default', 'image': {'position': u'left', 'imgsize': u'mini 200:200', 'visibility': u'on', 'order': u'5'}, 'date': {'order': u'6', 'visibility': u'on', 'format': u'datetime'}, 'autoplay': {'order': u'10', 'visibility': u'on'}}, 'plone.tiles.configuration.f416a80e-00ca-4971-8100-8f2c438ac1e4': {'css_class': u'tile-default', 'embed': {'order': u'1', 'visibility': u'on'}, 'description': {'order': u'3', 'visibility': u'on'}, 'title': {'htmltag': u'h2', 'order': u'2', 'visibility': u'on'}}, 'plone.tiles.configuration.f0d019f9-2113-4460-8d34-a4a73d5b2bee': {'css_class': u'tile-default', 'image': {'position': u'left', 'order': u'2', 'visibility': u'on', 'imgsize': u'mini 200:200'}, 'remote_url': {'visibility': u'on', 'order': u'3', 'htmltag': u'h2'}, 'title': {'visibility': u'on', 'order': u'1', 'htmltag': u'h2'}}, 'plone.tiles.configuration.d1a8beed-3d60-49ab-a121-18f3272ad5fc': {'description': {'order': u'2', 'visibility': u'on'}, 'title': {'htmltag': u'h2', 'order': u'0', 'visibility': u'on'}, 'css_class': u'tile-default', 'image': {'position': u'left', 'imgsize': u'mini 200:200', 'visibility': u'on', 'order': u'3'}, 'subjects': {'order': u'5', 'visibility': u'on'}, 'date': {'order': u'4', 'visibility': u'on', 'format': u'datetime'}}, 'plone.tiles.configuration.cdbea25a-9267-4ed1-88ac-e166521dcc74': {'css_class': u'tile-default', 'image': {'position': u'left', 'order': u'2', 'visibility': u'on', 'imgsize': u'mini 200:200'}, 'remote_url': {'htmltag': u'h2', 'order': u'3', 'visibility': u'on'}, 'title': {'htmltag': u'h2', 'order': u'1', 'visibility': u'on'}}, 'plone.tiles.configuration.a2f80390-e050-4d46-b860-5bff4b018730': {'description': {'order': u'3', 'visibility': u'on'}, 'footer': {'visibility': u'on', 'order': u'9', 'htmltag': u'h2'}, 'css_class': u'tile-default', 'image': {'position': u'left', 'imgsize': u'mini 200:200', 'visibility': u'on', 'order': u'5'}, 'title': {'htmltag': u'h2', 'order': u'2', 'visibility': u'on'}, 'header': {'htmltag': u'h2', 'order': u'1', 'visibility': u'on'}, 'offset': {'order': u'7', 'visibility': u'on', 'offset': u'0'}, 'date': {'order': u'4', 'visibility': u'on', 'format': u'datetime'}, 'number_to_show': {'order': u'6', 'visibility': u'on', 'size': u'5'}}, 'plone.locking': {u'plone.locking.stealable': {'token': '0.669065979675-0.748021800103-00105A989226:1510872362.434', 'type': <plone.locking.interfaces.LockType object at 0x7fdc988547d0>}}}
(Pdb) len(self.annotations)
8

this must grow exponentially with every checkout/checkin operation.

this is very serious and seems to me not easy to fix.

@rodfersou

This comment has been minimized.

Member

rodfersou commented Nov 17, 2017

IMHO the delete view should be removed, it don't make sense to do this kind of deletion while the layout is still not saved. It brings a bad behavior.. while you don't push the save button it is not expected to delete anything (this is MY understanding of this view).

The check should be moved into SAVE button, when you save the tile, you do a check if there is any annotation that don't belongs to any active tile, and remove it.

@hvelarde

This comment has been minimized.

Member

hvelarde commented Nov 17, 2017

the problem is tile configuration is saved on a tile basis; the Save button on the layout saves only changes to the layout itself.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment