Skip to content
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

PLIP: Fix and Improve Metadata Behaviors #2869

Open
iham opened this issue May 20, 2019 · 4 comments

Comments

@iham
Copy link
Member

commented May 20, 2019

PLIP (Plone Improvement Proposal)

Responsible Persons

Proposer: Markus Hilbert @iham

Seconder: Jens W. Klein @jensens

Abstract

Splitting up metadata/dublincore behaviors for higher flexibility and clean up their wrong implementations.

This PLIP targets Plone 6.

Motivation & Assumptions

1.) Wrong Implementation

  • the metadata behaviors seem to be implemented wrong as they do have a factory but not a marker-interface.
  • this leads to errors as neiter IDublinCore.providedBy(content_obj) nor IBasic.providedBy(content_obj) returns True

This needs to be fixed!

2.) Splitting Behaviors

plone.dublincore

Whenever you want to do anything on one of the most basic fields, you need to remove the plone.dublincore behavior from all standard contenttypes (plus additionals) and add those inside:

  • plone.basic
  • plone.ownership
  • plone.publication
  • plone.categorization

That is a lot of work just for a simple field exchange or small adjustments.

Even for bigger things, like adding a richtext description, interaction with the description (eg for cataloging) it is a lot of boilerplate.

plone.basic

This behavior deals with two cases, which - in our opinion - breaks with the basic idea of behaviors: Doing a small and (hopefully) simple task, being a single or even multiple fields, some functionality or something alike.

The construction of plone.basic "feels" locked-in.
This led to a rather weird implementation of the File and Image contenttypes:
Those have no plone.basic, but implement a XML schema with a title and description field.
The title is not mandatory/required, which is the only difference to plone.basic.

Having plone.basic split up into - lets say - ITitle (plone.title) and IDescription (plone.description), would solve a lot of those "issues" as we gain flexibility, overidability and reconfigurability.

In addition to that, we could/should have a title which is not required, to get rid of that separated XML implementation of File and Image.
Unfortunately there is no hook to configure a behavior in the FTI.

Therefore we would need two behaviors. So this would result in

  • IRequiredTitle -> plone.title.required
  • IOptionalTitle -> plone.title.optional

Which are our suggestions for distinct naming.

plone.categorization

This is another candidate (even it's not as important as above):

  • subjects aka tags (mixed up naming b/c Dublincore specification vs. user expectation) and
  • language

have nothing in common and don't interact besides sharing the same tab (fieldset).

Having plone.categorization split up into

  • plone.subjects (plone.tags?) and
  • plone.language

would be beneficial as described for plone.basic.

Proposal & Implementation

Fixing adapter/marker usage

Adapters and markers shall work as expected and as documented in plone.behavior and need to be fixed.

Splitting behaviors

  • replacements in the content types FTI's XML:

    <element value="plone.dublincore" />
    

    with

    <element value="plone.basic" />
    
    <element value="plone.ownership" />
    
    <element value="plone.publication" />
    
    <element value="plone.categorization" />
    

    or - if replacing plone.basic ist also a topic - with:

    <element value="plone.[title|required_title|optional_title]" />
    
    <element value="plone.description" />
    
    <element value="plone.ownership" />
    
    <element value="plone.publication" />
    
    <element value="plone.categorization" />
    

    or - if replacing plone.categorization ist also a topic - with:

    <element value="plone.[title|required_title|optional_title]" />
    
    <element value="plone.description" />
    
    <element value="plone.ownership" />
    
    <element value="plone.publication" />
    
    <element value="plone.[subjects|tags]" />
    
    <element value="plone.language" />
    
  • The new behaviors need to be implemented,

  • backward compatible behaviors need to be in place, issuing a deprecation warning if used.

  • an upgrade step need to be provided.

  • documentation and training needs to be reviews an adapted.

BBB Code

There are probably several places in third party code where IDublinCore(obj) adaption is used, which is fine so far.
There has to be be an adapter called IDublinCore to get all the fields provided by as is now.
In future this is an adapter, not a behavior as this only delivers data.
It will issue deprecation warnings follwoing Plone deprecation best practice when used.

Remark: Since IBasic(obj) and ICategorization(obj) is at the current state not working, there is no BBB adapter needed.

Deliverables

Plone Core

  • plone.app.contenttypes
  • plone.app.dexterity
  • plone.app.upgrade
  • documentation
  • training

Third party

  • bobtemplates.plone

Risks

Third party modules depending on the names or schema

  • plone.basic or IBasic,
  • plone.dublincore or IDublinCore,
  • plone.categorization or ICatgeorization

may break.

Since the Schema names are used by z3c.form as identifiers in the HTML code this may also affect custom styles.

Participants

@iham @jensens

@iham iham added this to the Plone 6.0 milestone May 20, 2019

@iham iham assigned iham and jensens May 20, 2019

@jensens jensens changed the title fix and improve metadata behaviors PLIP: Fix and Improve Metadata Behaviors May 21, 2019

@jensens jensens added this to Submitted (info complete) in PLIPs May 21, 2019

@MrTango

This comment has been minimized.

Copy link
Contributor

commented Jul 1, 2019

At least for this I'm all in, separated behaviors instead of dublincore is much cleaner and comon practis for a while now in addons:

  • Fixing adapter/marker usage
  • Splitting behaviors

I'm ok with the rest of the changes too but have not strong opinion on that.

@ale-rt

This comment has been minimized.

Copy link
Member

commented Sep 10, 2019

While I agree with fixing the adapter/marker usage I personally do not like the idea of shipping Plone code with so many granular behaviors.

Also I would like to defend the plone.dublincore behavior: it makes very much sense to me to have it pulled in with a single line than with many.

Then I perfectly acknowledge the problems that lead to replace plone.dublincore with more behaviors in the File and Image FTIs definition, but IMHO this solution is not optimal.

It seems to me that we always want the File and Image object to have a title.
That means we want them to obey to the default plone.duclincore schema definition.

The fact that the add and edit view do not require it is a completely different thing.

Talking MVCish:

  • the Model wants a Title (i.e. plone.dublincore)
  • the View does not care about the title
  • the Controller fills the gap by providing a fallback title if nothing was added

I hope you see my point here.

Then, as said in the beginning, I perfectly agree that the current behavior implementation needs some love.

@MrTango

This comment has been minimized.

Copy link
Contributor

commented Sep 10, 2019

The behaviors are all there, but they where bundled in dublincore behavior, which it self had no function.
There is no real benefit from using this behavior, but a couple of downsides.
If we just activate the 4 behaviors directly, everybody understands whats going on and can adjust it if she/he does not want parts of this default behavior of Plone.

In case you are talking about the second approache, i still like it more than forcing people to override programatically behaviors, because they are not flexible.

But yeah, i would like to have a more generic solution there too.
Maybe a beavior could read some settings from the registry HIDE :) ?

@ale-rt

This comment has been minimized.

Copy link
Member

commented Sep 10, 2019

If I got this PLIP right, it does want to add new behaviors.

The function of the plone.dublincore behavior is to declare that the objects respects the dublincore and so provides the dedicated expected accessors for the dublincore fields.
I see the point of having it.
I agree, the same result can be achieved pulling in several behavior at a time, but it is much more elegant to do it with just one line that with many.

If we extremize the "customizability approach" we will end up creating a behavior for each field.
Also I would discourage mixing the registry with the behaviors, we have XML model schemas or Python code for very custom things.

P.s.: I did not get what the HIDE acronym stands for :p

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
You can’t perform that action at this time.