feat(medusa): Nested Categories Admin Delete Endpoint #2975

riqwan · 2023-01-10T07:19:41Z

What:

Introduces an admin endpoint that allows a user to delete a product category, given an ID.

Why:

This is part of a greater goal of allowing products to be added to multiple categories.

How:

Creates a route on the admin scope to delete category
Creates a method in product category services to delete a category

RESOLVES CORE-957

changeset-bot · 2023-01-10T07:19:44Z

🦋 Changeset detected

Latest commit: 4a0a731

The changes in this PR will be included in the next version bump.

This PR includes changesets to release 1 package

Name	Type
@medusajs/medusa	Patch

Not sure what this means? Click here to learn what changesets are.

Click here if you're a maintainer who wants to add another changeset to this PR

adrien2p

Nice work 🎉

packages/medusa/src/services/product-category.ts

packages/medusa/src/api/routes/admin/product-categories/delete-product-category.ts

packages/medusa/src/services/__tests__/product-category.ts

adrien2p · 2023-01-10T08:44:44Z

packages/medusa/src/services/__tests__/product-category.ts

+        IdMap.getId("jeans")
+      )
+
+      expect(productCategoryRepository.softRemove).toBeCalledTimes(1)


question: I am wondering if it make sense to softRemove a category? Is it to allow a user to recover it?

No particular reason here. Just following the same methods as what other similar resources are doing (sales channels, product collection, etc.)

My question is more about, should a category be soft deletable?

cc @olivermrbl

packages/medusa/src/services/product-category.ts

pKorsholm

Very nice! I really like the thoroughness of the tests!

I have a comment regarding idempotent deletes as a convention, but otherwise I have no comments, it looks great! 😄

packages/medusa/src/services/product-category.ts

srindom

LGTM!

adrien2p

LGTM, just one change to make before merging

packages/medusa/src/services/product-category.ts

.changeset/olive-dots-whisper.md

olivermrbl · 2023-01-10T11:32:30Z

packages/medusa/src/services/product-category.ts

+        )
+      }
+
+      await productCategoryRepository.softRemove(productCategory)


comment(non-blocking): Bringing the discussion down here @adrien2p, we should be able to hard-delete categories, as we don't benefit from any of the usual advantages that come from soft-deletion

moved this to hard delete.

olivermrbl · 2023-01-10T11:33:33Z

packages/medusa/src/services/product-category.ts

+      const productCategory = await this.retrieve(productCategoryId, {
+        relations: ["category_children"],
+      })


todo: Retrieve will throw on non-existing categories, so to resolve idempotently we should catch the error and return undefined

Suggested change

const productCategory = await this.retrieve(productCategoryId, {

relations: ["category_children"],

})

const productCategory = await this.retrieve(productCategoryId, {

relations: ["category_children"],

}).catch(err => void 0)

.catch(() => void 0)

olivermrbl

LGTM!

adrien2p · 2023-01-10T13:47:30Z

packages/medusa/src/services/product-category.ts

+
+      const productCategory = await this.retrieve(productCategoryId, {
+        relations: ["category_children"],
+      }).catch((err) => void 0)


non blocking: remove err

adrien2p · 2023-01-10T13:47:43Z

packages/medusa/src/services/product-category.ts

+
+      await productCategoryRepository.delete(productCategory.id)
+
+      return Promise.resolve()


todo: remove, async already wrap in a promise

adrien2p

two minor comments

riqwan added 16 commits January 9, 2023 12:46

chore: added get route for admin categories API

3496726

chore: add tree method to mock repository

31fc4d5

Merge branch 'develop' into feat/nested-category-get-endpoint

9d7b3ff

chore: added changeset to the PR

2a7b371

chore: rename id to productCategoryId in service

818c3f3

Merge branch 'develop' into feat/nested-category-get-endpoint

3529ca9

chore: switch cli option to string

c628e61

chore: lint fixes, tests for parent category

2b6a91c

chore: move Nested Categories behind feature flag

158aa03

Merge branch 'develop' into feat/nested-category-get-endpoint

c5c0ae0

chore: use transformQuery hook in api

a610f15

chore: add feature flag in migrations

b5fa14a

Merge branch 'develop' into feat/nested-category-get-endpoint

0d1795b

chore: remove migration FF, fix FF name

393366c

Merge branch 'develop' into feat/nested-category-get-endpoint

ffb1f15

chore: added delete endpoint for categories

6b597bc

riqwan marked this pull request as ready for review January 10, 2023 07:21

riqwan requested a review from a team as a code owner January 10, 2023 07:21

riqwan requested review from adrien2p, carlos-r-l-rodrigues, olivermrbl and pKorsholm January 10, 2023 07:21

adrien2p requested changes Jan 10, 2023

View reviewed changes

pKorsholm reviewed Jan 10, 2023

View reviewed changes

packages/medusa/src/services/product-category.ts Show resolved Hide resolved

Base automatically changed from feat/nested-category-get-endpoint to develop January 10, 2023 09:08

riqwan added 2 commits January 10, 2023 14:52

chore: merge with latest develop

a038c31

chore: address pr comments

08b53c7

riqwan requested review from adrien2p and pKorsholm January 10, 2023 09:49

Merge branch 'develop' into feat/nested-category-delete-endpoint

99575b5

srindom approved these changes Jan 10, 2023

View reviewed changes

riqwan changed the title ~~feat(medusa): added delete endpoint for categories~~ feat(medusa): Nested Categories Admin Delete Endpoint Jan 10, 2023

adrien2p approved these changes Jan 10, 2023

View reviewed changes

packages/medusa/src/services/product-category.ts Outdated Show resolved Hide resolved

olivermrbl reviewed Jan 10, 2023

View reviewed changes

.changeset/olive-dots-whisper.md Outdated Show resolved Hide resolved

chore: use atomicphase transaction

8ed911b

riqwan force-pushed the feat/nested-category-delete-endpoint branch from 3e6b94c to 8ed911b Compare January 10, 2023 11:00

olivermrbl reviewed Jan 10, 2023

View reviewed changes

olivermrbl requested changes Jan 10, 2023

View reviewed changes

chore: merge with latest develop

dfb16b0

riqwan force-pushed the feat/nested-category-delete-endpoint branch 2 times, most recently from 10e1440 to 904b218 Compare January 10, 2023 12:24

riqwan requested a review from olivermrbl January 10, 2023 12:31

riqwan force-pushed the feat/nested-category-delete-endpoint branch from 904b218 to 2a32017 Compare January 10, 2023 12:36

chore: move to hard delete + safely traverse invalid ID

c16773b

riqwan force-pushed the feat/nested-category-delete-endpoint branch from 2a32017 to c16773b Compare January 10, 2023 12:36

olivermrbl approved these changes Jan 10, 2023

View reviewed changes

Merge branch 'develop' into feat/nested-category-delete-endpoint

4a0a731

olivermrbl added the automerge label Jan 10, 2023

adrien2p reviewed Jan 10, 2023

View reviewed changes

adrien2p approved these changes Jan 10, 2023

View reviewed changes

kodiakhq bot merged commit 71fa608 into develop Jan 10, 2023

kodiakhq bot deleted the feat/nested-category-delete-endpoint branch January 10, 2023 14:28

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

feat(medusa): Nested Categories Admin Delete Endpoint #2975

feat(medusa): Nested Categories Admin Delete Endpoint #2975

riqwan commented Jan 10, 2023 •

edited

Loading

changeset-bot bot commented Jan 10, 2023 •

edited

Loading

adrien2p left a comment

adrien2p Jan 10, 2023

riqwan Jan 10, 2023

adrien2p Jan 10, 2023

pKorsholm left a comment

srindom left a comment

adrien2p left a comment

olivermrbl Jan 10, 2023

riqwan Jan 10, 2023

olivermrbl Jan 10, 2023

adrien2p Jan 10, 2023

olivermrbl left a comment

adrien2p Jan 10, 2023

adrien2p Jan 10, 2023 •

edited

Loading

adrien2p left a comment


		await productCategoryRepository.delete(productCategory.id)

		return Promise.resolve()

feat(medusa): Nested Categories Admin Delete Endpoint #2975

feat(medusa): Nested Categories Admin Delete Endpoint #2975

Conversation

riqwan commented Jan 10, 2023 • edited Loading

changeset-bot bot commented Jan 10, 2023 • edited Loading

🦋 Changeset detected

adrien2p left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

pKorsholm left a comment

Choose a reason for hiding this comment

srindom left a comment

Choose a reason for hiding this comment

adrien2p left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

olivermrbl left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

adrien2p Jan 10, 2023 • edited Loading

Choose a reason for hiding this comment

adrien2p left a comment

Choose a reason for hiding this comment

riqwan commented Jan 10, 2023 •

edited

Loading

changeset-bot bot commented Jan 10, 2023 •

edited

Loading

adrien2p Jan 10, 2023 •

edited

Loading