asyavee/medusa-store
1
0
Fork 0
Code Issues Pull Requests Actions 2 Packages Projects Releases Wiki Activity

Merge pull request #2729 from medusajs/fix/discount-regions-validation

Browse Source 
fix(medusa): discounts service create validates against at-least 1 region
This commit is contained in:
Riqwan Thamir
2022-12-08 11:34:51 +01:00
committed by GitHub
parent 2b2ea157a6 dbe25245b5
commit 34e405c492
7 changed files with 242 additions and 99 deletions
Download Patch File Download Diff File Expand all files Collapse all files

5
.changeset/tender-coins-kiss.md Normal file
View File

@@ -0,0 +1,5 @@
---
"@medusajs/medusa": patch
---
Enforces a validation for discounts that makes regions a required parameter

199
integration-tests/api/__tests__/admin/discount.js
View File

@@ -24,6 +24,9 @@ const adminReqConfig = {
},
}
const validRegionId = "test-region"
const invalidRegionId = "not-a-valid-region"
describe("/admin/discounts", () => {
let medusaProcess
let dbConnection
@@ -408,6 +411,7 @@ describe("/admin/discounts", () => {
value: 10,
allocation: "total",
},
regions: [validRegionId],
usage_limit: 10,
},
adminReqConfig
@@ -471,6 +475,7 @@ describe("/admin/discounts", () => {
},
],
},
regions: [validRegionId],
usage_limit: 10,
},
adminReqConfig
@@ -538,6 +543,7 @@ describe("/admin/discounts", () => {
},
],
},
regions: [validRegionId],
usage_limit: 10,
},
adminReqConfig
@@ -576,6 +582,7 @@ describe("/admin/discounts", () => {
},
],
},
regions: [validRegionId],
},
adminReqConfig
)
@@ -630,6 +637,7 @@ describe("/admin/discounts", () => {
},
],
},
regions: [validRegionId],
usage_limit: 10,
},
adminReqConfig
@@ -654,6 +662,7 @@ describe("/admin/discounts", () => {
},
],
},
regions: [validRegionId],
},
{
headers: {
@@ -690,6 +699,7 @@ describe("/admin/discounts", () => {
},
],
},
regions: [validRegionId],
usage_limit: 10,
},
adminReqConfig
@@ -729,6 +739,7 @@ describe("/admin/discounts", () => {
},
],
},
regions: [validRegionId],
usage_limit: 10,
},
adminReqConfig
@@ -755,6 +766,7 @@ describe("/admin/discounts", () => {
},
],
},
regions: [validRegionId],
},
{
headers: {
@@ -779,6 +791,7 @@ describe("/admin/discounts", () => {
value: 10,
allocation: "total",
},
regions: [validRegionId],
usage_limit: 10,
},
adminReqConfig
@@ -822,6 +835,7 @@ describe("/admin/discounts", () => {
value: 10,
allocation: "total",
},
regions: [validRegionId],
usage_limit: 10,
},
adminReqConfig
@@ -844,6 +858,7 @@ describe("/admin/discounts", () => {
id: response.data.discount.rule.id,
type: "free_shipping",
},
regions: [validRegionId],
},
adminReqConfig
)
@@ -869,6 +884,7 @@ describe("/admin/discounts", () => {
value: 10,
allocation: "total",
},
regions: [validRegionId],
usage_limit: 10,
},
adminReqConfig
@@ -883,7 +899,7 @@ describe("/admin/discounts", () => {
})
)
await api
const err = await api
.post(
`/admin/discounts/${response.data.discount.id}`,
{
@@ -891,13 +907,12 @@ describe("/admin/discounts", () => {
is_dynamic: false,
},
adminReqConfig
)
.catch((err) => {
expect(err.response.status).toEqual(400)
expect(err.response.data.message).toEqual(
"property is_dynamic should not exist"
)
})
).catch(e => e)
expect(err.response.status).toEqual(400)
expect(err.response.data.message).toEqual(
"property is_dynamic should not exist"
)
})
it("automatically sets the code to an uppercase string on update", async () => {
@@ -913,6 +928,7 @@ describe("/admin/discounts", () => {
value: 10,
allocation: "total",
},
regions: [validRegionId],
usage_limit: 10,
},
adminReqConfig
@@ -958,6 +974,7 @@ describe("/admin/discounts", () => {
value: 10,
allocation: "total",
},
regions: [validRegionId],
usage_limit: 10,
},
adminReqConfig
@@ -991,10 +1008,9 @@ describe("/admin/discounts", () => {
})
it("fails to create a fixed discount with multiple regions", async () => {
expect.assertions(2)
const api = useApi()
await api
const err = await api
.post(
"/admin/discounts",
{
@@ -1007,20 +1023,18 @@ describe("/admin/discounts", () => {
allocation: "total",
},
usage_limit: 10,
regions: ["test-region", "test-region-2"],
regions: [validRegionId, "test-region-2"],
},
adminReqConfig
)
.catch((err) => {
expect(err.response.status).toEqual(400)
expect(err.response.data.message).toEqual(
`Fixed discounts can have one region`
)
})
).catch(e => e)
expect(err.response.status).toEqual(400)
expect(err.response.data.message).toEqual(
`Fixed discounts can have one region`
)
})
it("fails to update a fixed discount with multiple regions", async () => {
expect.assertions(2)
const api = useApi()
const response = await api.post(
@@ -1033,30 +1047,28 @@ describe("/admin/discounts", () => {
value: 10,
allocation: "total",
},
regions: [validRegionId],
usage_limit: 10,
},
adminReqConfig
)
await api
const err = await api
.post(
`/admin/discounts/${response.data.discount.id}`,
{
regions: ["test-region", "test-region-2"],
regions: [validRegionId, "test-region-2"],
},
adminReqConfig
)
).catch(e => e)
.catch((err) => {
expect(err.response.status).toEqual(400)
expect(err.response.data.message).toEqual(
`Fixed discounts can have one region`
)
})
expect(err.response.status).toEqual(400)
expect(err.response.data.message).toEqual(
`Fixed discounts can have one region`
)
})
it("fails to add a region to a fixed discount with an existing region", async () => {
expect.assertions(2)
const api = useApi()
const response = await api.post(
@@ -1070,24 +1082,22 @@ describe("/admin/discounts", () => {
allocation: "total",
},
usage_limit: 10,
regions: ["test-region"],
regions: [validRegionId],
},
adminReqConfig
)
await api
const err = await api
.post(
`/admin/discounts/${response.data.discount.id}/regions/test-region-2`,
{},
adminReqConfig
)
).catch(e => e)
.catch((err) => {
expect(err.response.status).toEqual(400)
expect(err.response.data.message).toEqual(
`Fixed discounts can have one region`
)
})
expect(err.response.status).toEqual(400)
expect(err.response.data.message).toEqual(
`Fixed discounts can have one region`
)
})
it("creates a discount with start and end dates", async () => {
@@ -1103,6 +1113,7 @@ describe("/admin/discounts", () => {
value: 10,
allocation: "total",
},
regions: [validRegionId],
usage_limit: 10,
starts_at: new Date("09/15/2021 11:50"),
ends_at: new Date("09/15/2021 17:50"),
@@ -1172,6 +1183,7 @@ describe("/admin/discounts", () => {
value: 10,
allocation: "total",
},
regions: [validRegionId],
usage_limit: 10,
starts_at: new Date("09/15/2021 11:50"),
ends_at: new Date("09/15/2021 17:50"),
@@ -1215,10 +1227,9 @@ describe("/admin/discounts", () => {
})
it("fails to create discount with end date before start date", async () => {
expect.assertions(2)
const api = useApi()
await api
const err = await api
.post(
"/admin/discounts",
{
@@ -1229,20 +1240,91 @@ describe("/admin/discounts", () => {
value: 10,
allocation: "total",
},
regions: [validRegionId],
usage_limit: 10,
starts_at: new Date("09/15/2021 11:50"),
ends_at: new Date("09/14/2021 17:50"),
},
adminReqConfig
)
.catch((err) => {
expect(err.response.status).toEqual(400)
expect(err.response.data).toEqual(
expect.objectContaining({
message: `"ends_at" must be greater than "starts_at"`,
})
)
).catch(e => e)
expect(err.response.status).toEqual(400)
expect(err.response.data).toEqual(
expect.objectContaining({
message: `"ends_at" must be greater than "starts_at"`,
})
)
})
it("fails to create a discount if the regions contains an invalid regionId ", async () => {
const api = useApi()
const err = await api.post(
"/admin/discounts",
{
code: "HELLOWORLD",
rule: {
description: "test",
type: "percentage",
value: 10,
allocation: "total",
},
regions: [validRegionId, invalidRegionId],
},
adminReqConfig
).catch(e => e)
expect(err.response.status).toEqual(404)
expect(err.response.data.message).toEqual(
`Region with not-a-valid-region was not found`
)
})
it("fails to create a discount if the regions contains only invalid regionIds ", async () => {
const api = useApi()
const err = await api.post(
"/admin/discounts",
{
code: "HELLOWORLD",
rule: {
description: "test",
type: "percentage",
value: 10,
allocation: "total",
},
regions: [invalidRegionId],
},
adminReqConfig
).catch(e => e)
expect(err.response.status).toEqual(404)
expect(err.response.data.message).toEqual(
`Region with not-a-valid-region was not found`
)
})
it("fails to create a discount if regions are not present ", async () => {
const api = useApi()
const err = await api.post(
"/admin/discounts",
{
code: "HELLOWORLD",
rule: {
description: "test",
type: "percentage",
value: 10,
allocation: "total",
},
},
adminReqConfig
).catch((e) => e)
expect(err.response.status).toEqual(400)
expect(err.response.data.message).toEqual(
`each value in regions must be a string, regions must be an array`
)
})
})
@@ -1306,7 +1388,10 @@ describe("/admin/discounts", () => {
let manager
beforeEach(async () => {
manager = dbConnection.manager
await adminSeeder(dbConnection)
await discountSeeder(dbConnection)
await manager.insert(DiscountRule, {
id: "test-discount-rule",
description: "Test discount rule",
@@ -1345,6 +1430,7 @@ describe("/admin/discounts", () => {
value: 10,
allocation: "total",
},
regions: [validRegionId],
usage_limit: 10,
},
adminReqConfig
@@ -1374,6 +1460,7 @@ describe("/admin/discounts", () => {
value: 10,
allocation: "total",
},
regions: [validRegionId],
usage_limit: 10,
},
adminReqConfig
@@ -2033,7 +2120,9 @@ describe("/admin/discounts", () => {
describe("GET /admin/discounts/code/:code", () => {
beforeEach(async () => {
const manager = dbConnection.manager
await adminSeeder(dbConnection)
await discountSeeder(dbConnection)
await manager.insert(DiscountRule, {
id: "test-discount-rule-fixed",
@@ -2115,15 +2204,12 @@ describe("/admin/discounts", () => {
it("should respond with 404 on non-existing code", async () => {
const api = useApi()
const err = await api.get("/admin/discounts/code/non-existing", adminReqConfig).catch(e => e)
try {
await api.get("/admin/discounts/code/non-existing", adminReqConfig)
} catch (error) {
expect(error.response.status).toEqual(404)
expect(error.response.data.message).toBe(
"Discount with code non-existing was not found"
)
}
expect(err.response.status).toEqual(404)
expect(err.response.data.message).toBe(
"Discount with code non-existing was not found"
)
})
it("should trim and uppercase code on insert", async () => {
@@ -2139,6 +2225,7 @@ describe("/admin/discounts", () => {
value: 10,
allocation: "total",
},
regions: [validRegionId],
usage_limit: 10,
},
adminReqConfig

96
packages/medusa/src/api/routes/admin/discounts/__tests__/create-discount.js
View File

@@ -2,7 +2,15 @@ import { IdMap } from "medusa-test-utils"
import { request } from "../../../../../helpers/test-request"
import { DiscountServiceMock } from "../../../../../services/__mocks__/discount"
const validRegionId = IdMap.getId("region-france")
describe("POST /admin/discounts", () => {
const adminSession = {
jwt: {
userId: IdMap.getId("admin_user")
}
}
describe("successful creation", () => {
let subject
@@ -16,14 +24,11 @@ describe("POST /admin/discounts", () => {
value: 10,
allocation: "total",
},
regions: [validRegionId],
starts_at: "02/02/2021 13:45",
ends_at: "03/14/2021 04:30",
},
adminSession: {
jwt: {
userId: IdMap.getId("admin_user"),
},
},
adminSession,
})
})
@@ -41,6 +46,7 @@ describe("POST /admin/discounts", () => {
value: 10,
allocation: "total",
},
regions: [validRegionId],
starts_at: new Date("02/02/2021 13:45"),
ends_at: new Date("03/14/2021 04:30"),
is_disabled: false,
@@ -54,6 +60,7 @@ describe("POST /admin/discounts", () => {
beforeAll(async () => {
jest.clearAllMocks()
subject = await request("POST", "/admin/discounts", {
payload: {
code: "TEST",
@@ -63,15 +70,12 @@ describe("POST /admin/discounts", () => {
value: 10,
allocation: "total",
},
regions: [validRegionId],
starts_at: "02/02/2021 13:45",
is_dynamic: true,
valid_duration: "PaMT2D",
},
adminSession: {
jwt: {
userId: IdMap.getId("admin_user"),
},
},
adminSession,
})
})
@@ -100,15 +104,12 @@ describe("POST /admin/discounts", () => {
value: 10,
allocation: "total",
},
regions: [validRegionId],
starts_at: "02/02/2021 13:45",
is_dynamic: true,
valid_duration: "P1Y2M03DT04H05M",
},
adminSession: {
jwt: {
userId: IdMap.getId("admin_user"),
},
},
adminSession,
})
})
@@ -126,6 +127,7 @@ describe("POST /admin/discounts", () => {
value: 10,
allocation: "total",
},
regions: [validRegionId],
starts_at: new Date("02/02/2021 13:45"),
is_disabled: false,
is_dynamic: true,
@@ -146,12 +148,9 @@ describe("POST /admin/discounts", () => {
value: 10,
allocation: "total",
},
regions: [validRegionId],
},
adminSession: {
jwt: {
userId: IdMap.getId("admin_user"),
},
},
adminSession,
})
})
@@ -186,15 +185,12 @@ describe("POST /admin/discounts", () => {
},
],
},
regions: [validRegionId],
starts_at: "02/02/2021 13:45",
is_dynamic: true,
valid_duration: "P1Y2M03DT04H05M",
},
adminSession: {
jwt: {
userId: IdMap.getId("admin_user"),
},
},
adminSession,
})
})
@@ -222,14 +218,11 @@ describe("POST /admin/discounts", () => {
value: 10,
allocation: "total",
},
regions: [validRegionId],
ends_at: "02/02/2021",
starts_at: "03/14/2021",
},
adminSession: {
jwt: {
userId: IdMap.getId("admin_user"),
},
},
adminSession,
})
})
@@ -259,13 +252,10 @@ describe("POST /admin/discounts", () => {
value: 10,
allocation: "total",
},
regions: [validRegionId],
starts_at: "03/14/2021 14:30",
},
adminSession: {
jwt: {
userId: IdMap.getId("admin_user"),
},
},
adminSession,
})
})
@@ -284,8 +274,44 @@ describe("POST /admin/discounts", () => {
value: 10,
allocation: "total",
},
regions: [validRegionId],
starts_at: new Date("03/14/2021 14:30"),
})
})
})
describe("unsuccessful creation with invalid regions", () => {
let subject
beforeAll(async () => {
jest.clearAllMocks()
subject = await request("POST", "/admin/discounts", {
payload: {
code: "TEST",
rule: {
description: "Test",
type: "fixed",
value: 10,
allocation: "total",
},
},
adminSession,
})
})
it("returns 400", () => {
expect(subject.status).toEqual(400)
})
it("returns error", () => {
expect(subject.body.message).toEqual(
`each value in regions must be a string, regions must be an array`
)
})
it("does not call service create", () => {
expect(DiscountServiceMock.create).toHaveBeenCalledTimes(0)
})
})
})

8
packages/medusa/src/api/routes/admin/discounts/create-discount.ts
View File

@@ -42,6 +42,7 @@ import { FindParams } from "../../../../types/common"
* required:
* - code
* - rule
* - regions
* properties:
* code:
* type: string
@@ -151,6 +152,7 @@ import { FindParams } from "../../../../types/common"
* value: 10,
* allocation: AllocationType.ITEM
* },
* regions: ['reg_XXXXXXXX'],
* is_dynamic: false,
* is_disabled: false
* })
@@ -169,7 +171,8 @@ import { FindParams } from "../../../../types/common"
* "type": "fixed",
* "value": 10,
* "allocation": "item"
* }
* },
* "regions": ['reg_XXXXXXXX']
* }'
* security:
* - api_token: []
@@ -257,9 +260,8 @@ export class AdminPostDiscountsReq {
usage_limit?: number
@IsArray()
@IsOptional()
@IsString({ each: true })
regions?: string[]
regions: string[]
@IsObject()
@IsOptional()

10
packages/medusa/src/helpers/test-request.js
View File

@@ -87,16 +87,18 @@ export async function request(method, url, opts = {}) {
)
headers.Cookie = headers.Cookie || ""
if (opts.adminSession) {
if (opts.adminSession.jwt) {
opts.adminSession.jwt = jwt.sign(
opts.adminSession.jwt,
const adminSession = { ...opts.adminSession }
if (adminSession.jwt) {
adminSession.jwt = jwt.sign(
adminSession.jwt,
config.projectConfig.jwt_secret,
{
expiresIn: "30m",
}
)
}
headers.Cookie = JSON.stringify(opts.adminSession) || ""
headers.Cookie = JSON.stringify(adminSession) || ""
}
if (opts.clientSession) {
if (opts.clientSession.jwt) {

16
packages/medusa/src/services/__tests__/discount.js
View File

@@ -9,7 +9,6 @@ const featureFlagRouter = new FlagRouter({})
describe("DiscountService", () => {
describe("create", () => {
const discountRepository = MockRepository({})
const discountRuleRepository = MockRepository({})
const regionService = {
@@ -54,6 +53,21 @@ describe("DiscountService", () => {
}
})
it("fails to create a discount without regions", async () => {
const err = await discountService.create({
code: "test",
rule: {
type: "fixed",
allocation: "total",
value: 20,
},
}).catch(e => e)
expect(err.type).toEqual("invalid_data")
expect(err.message).toEqual("Discount must have atleast 1 region")
expect(discountRepository.create).toHaveBeenCalledTimes(0)
})
it("successfully creates discount", async () => {
await discountService.create({
code: "test",

7
packages/medusa/src/services/discount.ts
View File

@@ -212,6 +212,13 @@ class DiscountService extends TransactionBaseService {
)) as Region[]
}
if (!discount.regions?.length) {
throw new MedusaError(
MedusaError.Types.INVALID_DATA,
"Discount must have atleast 1 region"
)
}
const discountRule = ruleRepo.create(validatedRule)
const createdDiscountRule = await ruleRepo.save(discountRule)