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

Support for sharing objects across groups of clients #2046

Closed
wants to merge 8 commits into from
Closed

Conversation

xispa
Copy link
Member

@xispa xispa commented Jul 6, 2022

Description of the issue/feature this PR addresses

With #2004 becomes possible to share an object with multiple clients. However, user had to manually and individually pick the clients to share the content with.

This Pull Request:

  • introduces the new content type ClientsGroup that makes the creation of client groups possible
  • allows the user to choose client groups for objects that implement IClientShareableBehavior behavior. An object shared with a client group becomes accessible to contacts from all clients that are assigned to the selected group

Current behavior before PR

Is not possible to share a given object with contacts from multiple clients in a single shot

Desired behavior after PR is merged

Is possible to share a given object with contacts from multiple clients in a single shot

--
I confirm I have tested this PR thoroughly and coded it according to PEP8
and Plone's Python styleguide standards.

@xispa xispa requested a review from ramonski July 6, 2022 15:43
Copy link
Contributor

@ramonski ramonski left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I believe that using objects for the purpose of grouping is the wrong approach and it clutters the core without additional value (if no patients add-on is installed).
In my opinion I would rather take the approach with an additional group field (or annotation) in clients and use a keyword index.
Adding clients to groups would then happen in the listing via a modal/view action to add/remove to groups.
This would have the additional value to be easy filterable in the listing, e.g. by clicking on a group and showing only the clients that belong to the same group (similar to the labels in GitHub issues).

<?xml version="1.0" encoding="UTF-8"?>
<object name="ClientsGroups"
meta_type="Dexterity FTI"
i18n:domain="senaite.diagnosis"
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

typo

class TypesAwareUIDReferenceField(UIDReferenceField):
"""A convenient field to handle UIDs of reference objects from different
types (e.g. Client + ClientsGroup) properly and in a more efficient way
"""
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I kind of understand what you are trying to do, but my brain turned off after 10 minutes looking at the logic of this field... Too many redirections

@xispa
Copy link
Member Author

xispa commented Jul 18, 2022

I believe that using objects for the purpose of grouping is the wrong approach and it clutters the core without additional value (if no patients add-on is installed). In my opinion I would rather take the approach with an additional group field (or annotation) in clients and use a keyword index. Adding clients to groups would then happen in the listing via a modal/view action to add/remove to groups. This would have the additional value to be easy filterable in the listing, e.g. by clicking on a group and showing only the clients that belong to the same group (similar to the labels in GitHub issues).

The approach you suggest by means of a new field with a "label"-like behavior looks promising. Will give a try, thanks.

@xispa xispa closed this Jul 18, 2022
@ramonski ramonski deleted the client-group branch February 23, 2023 08:58
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Development

Successfully merging this pull request may close these issues.

None yet

2 participants