Creme API #380
Draft
Creme API #380
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
genglert
requested changes
May 18, 2022
Comment on lines
+6
to
+8
| def get_cremeentity_contenttypes(): | ||
| models = list(creme_registry.iter_entity_models()) | ||
| return ContentType.objects.get_for_models(*models).values() |
There was a problem hiding this comment.
creme.creme_core.utils.content_type.entity_ctypes
Comment on lines
+50
to
+53
| fields = SimpleCremeEntitySerializer.Meta.fields + [ | ||
| "user", | ||
| "description", | ||
| ] |
There was a problem hiding this comment.
fields = [
*SimpleCremeEntitySerializer.Meta.fields,
"user",
"description",
]
(avoids a temporary list)
| class ContactSerializer(PersonWithAddressesMixin, CremeEntitySerializer): | ||
| class Meta(CremeEntitySerializer.Meta): | ||
| model = get_contact_model() | ||
| fields = CremeEntitySerializer.Meta.fields + [ |
There was a problem hiding this comment.
[*CremeEntitySerializer.Meta.fields, ...] (in others places too of course....)
Comment on lines
+65
to
+68
| created = CreationDateTimeField(verbose_name=_("Creation date"), editable=False) | ||
| modified = ModificationDateTimeField( | ||
| verbose_name=_("Last modification"), editable=False | ||
| ) |
There was a problem hiding this comment.
Indeed editable==False by default (yes I should simplify CremeEntity declaration too).
Comment on lines
+8
to
+47
| <style> | ||
| .api-content-page { | ||
| display: flex; | ||
| margin-top: 15px; | ||
| } | ||
| .api-left-panel-container { | ||
| flex-shrink: 1 | ||
| } | ||
| .api-left-panel-content { | ||
| min-width: 250px; | ||
| background: white; | ||
| border-left: 1px solid #d9d9d9; | ||
| border-right: 1px solid #d9d9d9; | ||
| border-top: 1px solid #d9d9d9; | ||
| border-bottom: 1px solid #dedede; | ||
|
|
||
| display: inline-block; | ||
|
|
||
| -webkit-box-shadow: 0 1px 2px rgba(0, 0, 0, 0.1); | ||
| -moz-box-shadow: 0 1px 2px rgba(0, 0, 0, 0.1); | ||
| box-shadow: 0 1px 2px rgba(0, 0, 0, 0.1); | ||
|
|
||
| } | ||
| .api-left-panel-content ul { | ||
| list-style: none; | ||
| } | ||
| .api-left-panel-content ul li { | ||
| padding-left: 15px; | ||
| } | ||
| .api-left-panel-content ul li.title { | ||
| padding-left: 0; | ||
| font-weight: 600; | ||
| padding-bottom: 10px; | ||
| } | ||
| .api-content-container { | ||
| flex: 1; | ||
| padding-left: 15px; | ||
| } | ||
|
|
||
| </style> |
There was a problem hiding this comment.
- I presume you plan to extract this to a separated CSS file
- Add a CSS class to the brick with {% block brick_extra_class %}{{block.super}} creme_api-brick{% endblock %} and it to prefix the CSS rules.
Comment on lines
+818
to
+837
| @property | ||
| def can_view(self): | ||
| return bool(self.value & EntityCredentials.VIEW) | ||
|
|
||
| @property | ||
| def can_change(self): | ||
| return bool(self.value & EntityCredentials.CHANGE) | ||
|
|
||
| @property | ||
| def can_delete(self): | ||
| return bool(self.value & EntityCredentials.DELETE) | ||
|
|
||
| @property | ||
| def can_link(self): | ||
| return bool(self.value & EntityCredentials.LINK) | ||
|
|
||
| @property | ||
| def can_unlink(self): | ||
| return bool(self.value & EntityCredentials.UNLINK) | ||
|
|
Comment on lines
+1310
to
+1315
| def refresh_from_db(self, *args, **kwargs): | ||
| self._teams = None | ||
| self._teammates = None | ||
| self._settings = None | ||
| super().refresh_from_db(*args, **kwargs) | ||
|
|
There was a problem hiding this comment.
Unit test (I presume it's better to test that queries are done with assertNumQueries() than testing protected attributes).
Comment on lines
+329
to
+342
| def refresh_from_db(self, *args, **kwargs): | ||
| self._allowed_apps = None | ||
| self._extended_allowed_apps = None | ||
|
|
||
| self._admin_4_apps = None | ||
| self._extended_admin_4_apps = None | ||
|
|
||
| self._creatable_ctypes_set = None | ||
| self._exportable_ctypes_set = None | ||
|
|
||
| self._setcredentials = None | ||
|
|
||
| super().refresh_from_db(*args, **kwargs) | ||
|
|
There was a problem hiding this comment.
Unit test (I presume it's better to test that queries are done with assertNumQueries() than testing protected attributes).
Comment on lines
+8
to
+22
| class ContentTypeViewSet(viewsets.ReadOnlyModelViewSet): | ||
| """ | ||
| retrieve: | ||
| Retrieve a content type. | ||
|
|
||
| list: | ||
| List content types. | ||
| """ | ||
|
|
||
| queryset = None | ||
| serializer_class = ContentTypeSerializer | ||
| schema = AutoSchema(tags=["Content Types"]) | ||
|
|
||
| def get_queryset(self): | ||
| return get_cremeentity_contenttype_queryset() |
There was a problem hiding this comment.
- Is it possible to return a list not based on a Queryset ? (if yes, remove 'get_cremeentity_contenttype_queryset()' )
- Rename EntityContentTypesViewSet VS return all ContentTypes (or entity types + types registered on crme_config + some types in an attributes with Relation/EntityFilters/...). I don't know where this view is used so I ignore what's the smartest.
Comment on lines
+114
to
+141
| class DeleteUserSerializer(serializers.ModelSerializer): | ||
| """ | ||
| Serializer which assigns the fields with type CremeUserForeignKey | ||
| referencing a given user A to another user B, then deletes A. | ||
| """ | ||
|
|
||
| transfer_to = serializers.PrimaryKeyRelatedField( | ||
| queryset=CremeUser.objects.none(), required=True | ||
| ) | ||
|
|
||
| class Meta: | ||
| model = CremeUser | ||
| fields = ["transfer_to"] | ||
|
|
||
| def __init__(self, instance=None, **kwargs): | ||
| super().__init__(instance=instance, **kwargs) | ||
| users = CremeUser.objects.exclude(is_staff=True) | ||
| if instance is not None: | ||
| users = users.exclude(pk=instance.pk) | ||
| self.fields["transfer_to"].queryset = users | ||
|
|
||
| def save(self, **kwargs): | ||
| CremeUserForeignKey._TRANSFER_TO_USER = self.validated_data["transfer_to"] | ||
|
|
||
| try: | ||
| self.instance.delete() | ||
| finally: | ||
| CremeUserForeignKey._TRANSFER_TO_USER = None |
There was a problem hiding this comment.
Beware the view 'creme.creme_config.views.user.UserDeletion' uses a lock to avoid race conditions (eg: a user transfers A to B, & other one transfers B to A).
- use the same lock (the name is in a class attribute)
- I presume we should perform the deletion in a deletion job (I mean in the creme_config too) ; but I do not know how the REST API would manage that (return immediately "the job has been created" VS wait the job is complete).
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
This is a work in progress