[Product-Developers] Questions about collective.cover

Martin Aspeli optilude+lists at gmail.com
Sat Sep 22 06:57:41 UTC 2012


Hi,

On 21 September 2012 16:07, Franco Pellegrini <frapell at gmail.com> 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.

>>   - 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.

>> 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.

>>  - 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 ;-)

>>  - 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


More information about the Product-Developers mailing list