Collection methods 1.0.x

[mmoravcik]

This ended-up only one method - a way of adding a content object to a collection/moderation request

[czpython]

use relative import

[czpython]

rename to add_content_object or add_object

[damilare]

@czpython We decided to use the name create_moderation_request_from_content_object() to make it explicit that we are creating a ModerationRequest, and not a ContentObject here, i.e ModerationRequest.objects.get_or_create().

This is the data flow we are following as far as I understand content_object (via content_type framework) -> moderation_request -> collections.

Naming this method add_content_object() implies that we are adding content_objects directly to the collections, which is not the case. If we want to treat moderation_request like a sort of tracking object, thereby making it silent or transparent, then, I'd prefer a method name like add_content_object_from_moderation_request

In any case, we shouldn't be inadvertently treating moderation_request like a content_object, or making it silent in the relationship between content_objects and collections.

[czpython]

I find that being super explicit in these cases leads to more confusion.
ModerationRequest is an implementation detail as such I don't see why we need to expose it.

[damilare]

I see your perspective, I was of the impression that ModerationRequest is at core to of djgangocms_moderation, as it capture the states of the objects, i.e approved, rejected. Makes sense to me.

[mmoravcik]

Reverting back to add_object

[czpython]

use self.moderation_requests.create()

[czpython]

this check should be on a helper function

[mmoravcik]

I am not sure I am following this. Do you mean something like creating a new get_content_type helper function, or is your comment talking about if self.is_locked which is above?

[czpython]

the check if moderation request for x object exists

[mmoravcik]

Hmm, it is a requirement that object is never part of another active collection, so I think it should be here to raise an Exception if that's the case?
We could add an optional force argument which will ignore is_locked and also existing requests check

[mmoravcik]

OK I've made the changes as discussed

[mmoravcik]

@czpython looks like that the tests are broken due to django-cms/django-cms@83d38db and AldrynForms

  File "/Users/fantiago/.virtualenvs/moderation/lib/python3.6/site-packages/aldryn_forms/models.py", line 9, in <module>
    from cms.utils.plugins import build_plugin_tree, downcast_plugins
ImportError: cannot import name 'build_plugin_tree'

[czpython]

Looks good.
At some point the check for existing requests will have to change because of archived collections