feat(order): module foundation #6399
Conversation
🦋 Changeset detectedLatest commit: 900fc71 The changes in this PR will be included in the next version bump. This PR includes changesets to release 3 packages
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 |
|
The latest updates on your projects. Learn more about Vercel for Git ↗︎
3 Ignored Deployments
|
| return { | ||
| toString: () => { | ||
| return expression | ||
| }, | ||
| valueOf: () => { | ||
| return expression | ||
| }, | ||
| name, | ||
| expression, | ||
| MikroORMIndex: (options = {}) => { | ||
| return Index({ | ||
| name, | ||
| expression, | ||
| ...options, | ||
| }) | ||
| }, | ||
| } |
There was a problem hiding this comment.
question: What's the idea here? And why did we change the fulfillment (among others) module indexes? Seems weird to have a discrepancy here.
There was a problem hiding this comment.
The idea is to be able to construct the index object and immediately apply it to the props without having to explicitly declared a name and build the expression and then having to apply them all.
const Index = createPsqlIndexHelper({
name: "IDX",
...
})
class Entity {
@Index.MikroORMIndex()
prop: something
}instead of
const indexName = 'IDX'
const index = createPsqlIndexHelper({
name: indexName,
...
})
class Entity {
@Index({
name: indexName,
expression: index
})
prop: something
}There was a problem hiding this comment.
Sorry, I realise I phrased my question wrong. Why do we need the different return values and why are we not using it the same why in the modules?
There was a problem hiding this comment.
You can have a need for the values for example like in the customer module where you need to index name in order to handle the error manually, in that case this index is exported and you can extract the name from it. The expression could be also needed why not. The idea was to not fully changed the other modules in order to reduce the amount of conflict. So the other modules are just extracting the expression so that only the indexes are touched and not the entities
There was a problem hiding this comment.
I didn't want to change the default behavior of the helper, and regardless the ORM used, the expression in plain sql might be needed.
I will refactor all the other modules in a next PR. as Adrien said, I'm avoiding a big number of conflicts for now.
| ): Promise<OrderTypes.OrderLineItemDTO[]> | ||
|
|
||
| @InjectManager("baseRepository_") | ||
| async addLineItems( |
There was a problem hiding this comment.
thought: we should probably have a check here eventually that verifies that the order is in a state where it's allowed to have line items added externally. For example, once the order is placed adding line items should go through the mechanism we come up with for edits.
There was a problem hiding this comment.
Similar thinking for some of the other functions.
There was a problem hiding this comment.
I agree, I am just avoiding in this PR handle things that are uncertain.
When we define the order edits, and what will be the rules, all these services will have to be revisited anyway.
In theory we could have multiple versions of the same order and the service will have to handle/validate that.
Same for the field placed_at. I will wait more definition how we'll manage order changes to change that.
| @@ -0,0 +1,31 @@ | |||
| /** | |||
There was a problem hiding this comment.
q: shouldn't this be in types?
There was a problem hiding this comment.
unfortunately enums are transpiled to objects. and things that generate code we have to move to utils.
There was a problem hiding this comment.
the approach so far is having an enum in the utils and a type in the types
olivermrbl
changed the title
What:
Initial Order module foundation.
Basic models and methods to manage entites like in Cart Module