Make senaite.jsonapi to be portal-type-naive and allow adapter for create operation

[xispa]

Description of the issue/feature this PR addresses

This pull request makes senaite.jsonapi to be portal_type-naive. This means that by default, senaite.jsonapi delegates the responsibility of creation operation to the underlying add-on where the given portal type is registered, so it no longer checks explicitly for portal types on creation.

Nevertheless, senaite.jsonapi does not allow the creation of objects when:

• the container is the portal root (senaite path)
• the container is senaite's setup (senaite/bika_setup path)
• the container does not allow the specified portal_type

For the cases above, senaite.jsonapi will always return a 401 response.

In addition, this Pull Request also allows to easily adapt the create operation for any given portal type and container. Sometimes, one might want to handle the creation of a given object differently,
either because:

• you want a portal type to never be created through senaite.jsonapi
• you want a portal type to only be created in some specific circumstances
• you want to add some additional logic within the creation process
• etc.

SENAITE.JSONAPI provides the ICreate interface that allows to handle the create operation with more granularity. An Adapter of this interface is initialized with the container object to be created. Example:

    from Products.CMFPlone.utils import _createObjectByType
    from senaite.jsonapi.interfaces import ICreate
    from zope import interface


    class TodoCreateAdapter(object):
        """Custom adapter for the creation of Todo type
        """
        interface.implements(ICreate)

        def __init__(self, container):
            self.container = container

        def is_creation_allowed(self):
            """Returns whether the creation of the portal_type is allowed
            """
            return True

        def is_creation_delegated(self):
            """Returns whether the creation of this portal type has to be
            delegated to this adapter
            """
            return True

        def create_object(self):
            """Creates an object
            """
            obj = _createObjectByType("Todo", self.container, tmpID())
            obj.edit(**data)
            obj.unmarkCreationFlag()
            obj.reindexObject()
            return obj

    <configure
        xmlns="http://namespaces.zope.org/zope">

        <!-- Adapter for custom creation of Todo -->
        <adapter
          name="Todo"
          factory=".TodoCreateAdapter"
          provides="senaite.jsonapi.interfaces.ICreate"
          for="bika.lims.interfaces.IClient" />

    </configure>

With this example, senaite.jsonapi will not follow the default procedure of creation, it will rather delegate the operation of the Todo object to the function create_object of this adapter. Note the creation will only be delegated when the function is_creation_delegated returns True.

Also note that if the function is_creation_allowed returns False, the system will raise a 401 exception.

Current behavior before PR

senaite.jsonapi is not portal type naive and does not allow to manually handle the creation of objects unless creating a new route.

Desired behavior after PR is merged

senaite.jsonapi becomes portal type naive and allows to manually handle the creation of specific objects without the need of creating an additional route.

--
I confirm I have tested the PR thoroughly and coded it according to PEP8
standards.

[ramonski]

Thanks for this PR! Pretty cool to adapt the object creation as well.

However, there's one caveat we need to address, which is the location/container where the object need to be created.

Currently, this is done in a hard coded way here:
https://github.com/senaite/senaite.jsonapi/pull/37/files#diff-aa9c49f3316ea3b1a902860e227779c6R1069

Maybe it would make sense to provide that in the adapter as well?

Issues occur e.g. for the type "DynamicAnalysisSpec" which I used for testing with that payload:

{
  "parent_path":"/senaite/bika_setup/dynamic_analysisspecs",
  "portal_type": "DynamicAnalysisSpec",
  "title": "My Spec"
}

It ended up in a renamed Setup folder:

[ramonski]

Hmm, I see that it should actually be the parent_path 🤔
Something is weird here. Would you mind to check that please with that Dexterity Type?

[xispa]

Hmm, I see that it should actually be the parent_path thinking
Something is weird here. Would you mind to check that please with that Dexterity Type?

Sure. Will check and revert back

[xispa]

However, there's one caveat we need to address, which is the location/container where the object need to be created.
Currently, this is done in a hard coded way here:
https://github.com/senaite/senaite.jsonapi/pull/37/files#diff-aa9c49f3316ea3b1a902860e227779c6R1069

I would entirely remove this "automatic" assignment of the parent path. I believe we should be as much coherent as possible. I mean, why user needs to set the parent container (via parent_path or uid) for some portal types while this is not necessary for others?. Also, there are some types that could be created in different places (e.g. SamplePoint type can be created either inside a Client or inside Setup). In addition, keep in mind that senaite.jsonapi should be as much agnostic as possible re types, mostly because you might have different add-ons, not only senaite.core. Therefore I would remove that. What do you think?

Maybe it would make sense to provide that in the adapter as well?

Adapter could be created, but if we entirely remove that auto-assignment, I feel it wouldn't provide any additional value

[xispa]

Hmm, I see that it should actually be the parent_path thinking
Something is weird here. Would you mind to check that please with that Dexterity Type?

Just found out what happens here... really weird. The problem is in core's api, here:

https://github.com/senaite/senaite.core/blob/dc27eb49b716eb435418d7d8ccbba4563fe6746d/bika/lims/api/__init__.py#L164

.... but obj is not the container!. Look:

 164         # Edit after processForm; processForm does AT unmarkCreationFlag.                          
 165  ->     obj.edit(**kwargs)                                                                         
 166                                                                                                    
 167         # explicit notification                                                                    
 168         modified(obj)                                                                              
 169         return obj                                                                                 
(Pdb++) container.title
u'Dynamic Analysis Specs'
(Pdb++) n                                                                 
(Pdb++) container.title
u'New DynamicAnalysisSpec'
(Pdb++) obj
<DynamicAnalysisSpec at /senaite/bika_setup/dynamic_analysisspecs/dynamicanalysisspec-11>
(Pdb++) container
<DynamicAnalysisSpecs at /senaite/bika_setup/dynamic_analysisspecs>

How this can be?

Maybe DynamicAnalysisSpec does not have title and then is set via acquisition chain?

The Dynamic Analysis Spec is created correctly, btw:

[ramonski]

I think the issue with the title is that it comes in via a behavior for dexterity contents:
https://github.com/senaite/senaite.core/blob/master/bika/lims/profiles/default/types/DynamicAnalysisSpec.xml#L54
Weird, we need to handle that somehow in core's create API.

[ramonski]

However, there's one caveat we need to address, which is the location/container where the object need to be created.
Currently, this is done in a hard coded way here:
https://github.com/senaite/senaite.jsonapi/pull/37/files#diff-aa9c49f3316ea3b1a902860e227779c6R1069

I would entirely remove this "automatic" assignment of the parent path. I believe we should be as much coherent as possible. I mean, why user needs to set the parent container (via parent_path or uid) for some portal types while this is not necessary for others?. Also, there are some types that could be created in different places (e.g. SamplePoint type can be created either inside a Client or inside Setup). In addition, keep in mind that senaite.jsonapi should be as much agnostic as possible re types, mostly because you might have different add-ons, not only senaite.core. Therefore I would remove that. What do you think?

Agree, we can remove that entirely. I think I added it to omit the parent_path or uid dance when adding setup items, but not sure anymore.

Maybe it would make sense to provide that in the adapter as well?

Adapter could be created, but if we entirely remove that auto-assignment, I feel it wouldn't provide any additional value

Yes, agree. Let's keep things simple.