On Sat, Sep 22, 2012 at 3:57 AM, Martin Aspeli <[email protected]>wrote:
> Hi, > > On 21 September 2012 16:07, Franco Pellegrini <[email protected]> wrote: > > >> - Why do we need IPersistentCoverTile? > > > > > > IPersistentCoverTile is the interface that base.PersistentCoverTile > > implements. This class inherits from PersistentTile and implement a > couple > > of things on top that we needed in cover. > > My concern is that it feels like we now have "cover tiles" and "other > tiles" and they're not compatible. So, you couldn't place a "cover > tile" in Deco because Deco wouldn't know how to configure it and you > couldn't place a standard tile in cover because it wouldn't have the > configuration it needed. This doesn't seem like a good outcome. > > Maybe it doesn't matter: if cover is about a fairly specific use case, > then there may not be much scope for tile reuse anyway. > That's about right, they are not compatible, however, i think c.cover needs really little tweaking in order to make "any tile" to work with it (that is without the extra features provided, like permissions and ability to drop content on them), and to make "c.cover tiles" work with Deco. And i don't think it is a bad outcome, c.cover was built to fulfill use cases not covered by current tiles implementation... sure, it's always better to get additional features, like this kind of compatibility, but at the moment, and even now, it doesn't seem to be a problem for any of the projects... > > >> - Why do we have so many get_* methods in IPersistentCoverTile? Why > >> not just access through the tile data API? > > > > > > "get_tile_configuration" is a helper method that will get the > configuration > > for a tile. Remember from my other mail that i told you i implemented a > > configure view similar to the edit one, where you could save specific > > configurations for fields ? well, using this you get that config. > > I need to understand that better, I think. > > >> - Suggestion: Instead of storing allowed/disallowed groups to edit a > >> tile on the tile itself, maybe delegate this to an adapter? It would > >> allow c.cover to work with tiles not derived from its special > >> IPersistentCoverTile > > > > > > I thought of doing this the same way as the data and configuration, that > > way, if we remove the tile, then we remove all this info too... if we do > it > > with an adapter, then we would need to write additional code so we do not > > end up with inconsistent info. > > You can store it in the same place, even if the API is exposed via an > adapter. > Right, but what would be the advantage of changing it to be an adapter ? > > >> Side note: I have a hope that most tiles will not need to be > >> persistent. There is something very clean and lightweight about only > >> storing configuration parameters in the query string in a tile href, > >> and annotations are somewhat opaque. > > > > > > I agree, but this data (in some situations) need to be saved somewhere, > > annotation, utility, etc... > > The point is that for a lot of tiles, all the config is primitive > values (strings, numbers). In this case, we can store in the query > string where the tile href is embedded, making for a fully transient > tile. This means no ZODB objects being created or read, so it is fast, > and no chance of cruft in the ZODB if tiles are moved around or > deleted. > I agree, however, i see a couple of issues with this, like URI lenght, and the fact that those values have to be stored somehow, somewhere, or maybe they can even be a result of some processing of context data > > >> - Why does IBasicTileData have get_* methods? Why not just access > >> through the tile data API? > > > > > > I don't like to access attributes directly, but to have methods to do it. > > This way if in the future field names change, or maybe we need to > preprocess > > or whatever the info before returning it (like for instance RichText), > you > > only need to do it in one place. I know this is not very "pythonic", old > > habits... > > This is what property() is for ;-) > I know, but i just don't like it very much, it can make the code a bit more complicated to understand, and i think it isn't much more different of doing it the way it is done, am i missing some theory behind it maybe ? > > >> - Same about the edit form in edit.py - what is missing from p.a.tiles? > > > > > > Since p.a.tiles does not implement permissions, and we did, then I had to > > override this... > > Maybe it should. What permissions are you managing, specifically? > > Martin > The idea is you can specify which groups are allowed to edit the tile's content, so we are not checking specific permissions, just checking if the user is in the proper group and raising Unauthorized... Kind regards, Franco
_______________________________________________ Product-Developers mailing list [email protected] https://lists.plone.org/mailman/listinfo/plone-product-developers
