Replace storage.create_annotation with an AnnotationWriteService
#7966
Conversation
|
|
||
| return annotation | ||
|
|
||
| def _validate_group(self, annotation: Annotation): |
There was a problem hiding this comment.
This is structured a bit differently from the original service, as the create and update methods were doing nearly the same thing. Pushing more functionality here allows it to be tested once and re-used.
|
|
||
| self._search_index_service._queue.add_by_id( # pylint: disable=protected-access | ||
| annotation.id, tag="storage.create_annotation", schedule_in=60 | ||
| ) |
There was a problem hiding this comment.
Why is the queue private?
| )._queue.add_by_id(annotation.id, tag="storage.create_annotation", schedule_in=60) | ||
|
|
||
| return annotation | ||
|
|
There was a problem hiding this comment.
As the _validate_group method is still used by update_annotation we don't get a size reduction proportionate to those we added. In the next PR when we remove the update method, a lot more should go.
| "links_service", | ||
| "annotation_json_service", | ||
| "storage", | ||
| ) | ||
| class TestCreate: |
There was a problem hiding this comment.
I've rewritten these tests as they were written in the style where every aspect of the method had it's own test, which made it very long.
jon-betts
force-pushed
the
anno-write-svc-create
branch
from
123f4d9
to
6dd5530
Compare
jon-betts
changed the title
AnnotationWriteService with create annotation methodstorage.create_annotation with an AnnotationWriteService method
jon-betts
changed the title
storage.create_annotation with an AnnotationWriteService methodstorage.create_annotation with an AnnotationWriteService
jon-betts
force-pushed
the
anno-write-svc-create
branch
from
6dd5530
to
6c02aa7
Compare
jon-betts
force-pushed
the
anno-write-svc-create
branch
from
6c02aa7
to
85e6b8d
Compare
| @@ -78,7 +79,9 @@ def create(request): | |||
| schema = CreateAnnotationSchema(request) | |||
| appstruct = schema.validate(_json_payload(request)) | |||
|
|
|||
| annotation = storage.create_annotation(request, appstruct) | |||
| annotation = request.find_service(AnnotationWriteService).create_annotation( | |||
| data=appstruct | |||
There was a problem hiding this comment.
I've seen this name before (appstruct) in the H code base and not sure what it means.
I wound't change it now thought. It's probably "idiomatic H".
There was a problem hiding this comment.
Yeah it probably is, but I agree it's off-putting.
h/services/annotation_write.py
Outdated
| db_session=request.db, | ||
| has_permission=request.has_permission, | ||
| search_index_service=request.find_service( | ||
| # pylint: disable=protected-access |
There was a problem hiding this comment.
What line is the disable targeting?
There was a problem hiding this comment.
I think something has gotten lost in the merging here...
jon-betts
force-pushed
the
anno-read-svc-fetch-all
branch
from
d4bf030
to
c124650
Compare
jon-betts
force-pushed
the
anno-write-svc-create
branch
from
85e6b8d
to
f06bb09
Compare
jon-betts
force-pushed
the
anno-write-svc-create
branch
from
f06bb09
to
aa848a2
Compare
For:
Requires:
storage.fetch_ordered_annotationwith anAnnotationReadServicemethod #7948Testing notes
For H / Via / ViaHTML / Client: