collections: improve service and resource apis by jrcastro2 · Pull Request #110 · inveniosoftware/invenio-collections

jrcastro2 · 2026-01-15T15:28:04Z

Complete resource layer with missing collections features
Enhance service layer
Add tests
Improve schema
closes 5 - Implement full collections management + Browse tab visibility rules CERNDocumentServer/cds-rdm#632
NEEDS feat: adds collection CRUD ui invenio-communities#1347 for tests permissions

egabancho · 2026-01-26T10:53:46Z

@jrcastro2, I am pretty biased on this one 😂 Probably somebody from the CDS/Zenodo team should look into it.

ptamarit · 2026-02-13T14:51:56Z

+        ctree=tree,
+    )
+
+    # Try to delete without cascade should fail


Do we allow users to try to delete without cascade if there are children in the UI?

Yes, the test there is for the service, the func requires the cascade param, the tests are meant so that if not passed it doesn't allow to delete the collection if there are children.

ptamarit · 2026-02-13T14:53:20Z

                path=path,
                title=title,
-                search_query=query,
+                search_query=search_query,


What is the thinking behind renaming query to search_query?
Does it break backwards-compatibility?

The reason for the rename is that IMHO it was wrong from before. In the Collection API obj we do have both, query property and the search_query, they are different things and at some point we are calling self.query and self.search_query, which could lead to confusion and it was actually causing issues for me so I decided to align things. The backwards compatibility would break if there is external code calling the Collection.create() (which I don't think should be the case). Other than that should be fine specially since it was a feature that was not fully used, but let me know if I am missing something.

kpsherva · 2026-03-17T13:25:29Z

+
+# revision identifiers, used by Alembic.
+revision = "3c1b8d0e7f52"
+down_revision = "d2f434c0ac92"


@palkerecsenyi @jrcastro2 is this the correct revision?

Thanks for checking, this looks correct. This is the latest revision

kpsherva · 2026-03-17T15:39:43Z

+import DeleteCollectionAction from "./actions/DeleteCollectionAction";
+import DeleteCollectionTreeAction from "./actions/DeleteCollectionTreeAction";
+
+class CollectionTreeCardGroup extends Component {


this component also needs responsibilities separation. Some of the states of the forms are belonging to child forms components, it is not entirely understandable to me why this parent component is evaluating these internal "children states"?
The component is very complex and needs simplifications in my opinion

The reason the parent holds the form state is cuz of the change to use modal actions (on the demo it was suggested to use them), this means that the Save button lives in Modal.Actions but Formik is in the child, so we lift just { isValid, isSubmitting, handleSubmit } via onFormReady. I believe that the same approach used in other places.

In any case I moved each modal to single cmp so that the state of each is not in the parent and like that it might be a bit more understandable but still each parent modal holds the stat.

Still this cmp is quite complex since it has the darg logic as well, let me know if this is good enough or if we should try to move this away as well.

slint

Minor comments on the backend parts, some shelveable.

* Complete resource layer with missing collections features * Enhance service layer * Add tests * Improve schema Co-Authored-By: egabancho <me@egabancho.com>

* closes CERNDocumentServer/cds-rdm#632

…y_id to namespace_id

palkerecsenyi · 2026-03-24T09:10:29Z

+    collection_tree_cls = CollectionTree
+    tree_schema = CollectionTreeSchema

    links_item = {


Are there no other links?

Good catch! I intentionally left just search for now because adding self_html requires the community slug, which isn't directly on the Collection model anymore (only namespace_id). I think this should be tackled when there is more work done on abstracting invenio-collections.

palkerecsenyi · 2026-03-24T09:14:34Z

+                )
+                .scalar()
+            )
+            data["order"] = (max_order or 0) + 10


The reason to use 10 comes from the initial implementation where the order was a form field and the user could input it. It would be easier to insert a new value in a middle position if we use tens, otherwise we would need to update all the other values. Right now we mostly insert at the end by default and let the user reorder the values which ends up reassigning the order to all the values. So this is not that much needed still I believe is a nice addition to keep then separated by tens.

jrcastro2 force-pushed the improve-collections-api-master branch 2 times, most recently from 1ba3ad8 to 091ea25 Compare January 15, 2026 16:45

ptamarit approved these changes Feb 13, 2026

View reviewed changes

jrcastro2 force-pushed the improve-collections-api-master branch 4 times, most recently from e4b7426 to 7a43162 Compare February 23, 2026 12:50

jrcastro2 force-pushed the improve-collections-api-master branch 2 times, most recently from 13442e0 to b567c6e Compare March 9, 2026 18:16