Consolidate api type definitions

[mattyg]

Currently we have many different type definitions for a single kind of data model. This adds a lot of unnecessary complexity in the backend that make it cumbersome to introduce minor changes.

For example -- on the praise model we have:

• Praise
• PraiseDocument
• PraiseDto
• PraiseDtoExtended
• PraiseDetailsDto

We should seek to consolidate these type definitions and their usage to reduce complexity and development overhead -- even if it means we're fetching slightly more data than needed on the frontend.

[kristoferlund]

• If I remember correctly, PraiseDtoExtended is used only locally in one of the endpoints.
• PraiseDetailsDto most likely can be merged with PraiseDto.
• Praise I guess could be merged with PraiseDocument. But wouldn't it lead to conflicts in places where we don't expect to see all the mongoose stuff?
• PraiseDto I do think is wise to maintain as a separate interface so that we have explicit control over what gets sent to the client.

[mattyg]

• If I remember correctly, PraiseDtoExtended is used only locally in one of the endpoints.
• PraiseDetailsDto most likely can be merged with PraiseDto.
• Praise I guess could be merged with PraiseDocument. But wouldn't it lead to conflicts in places where we don't expect to see all the mongoose stuff?
• PraiseDto I do think is wise to maintain as a separate interface so that we have explicit control over what gets sent to the client.

PraiseDetailsDto, PraiseDto, and PraiseDtoExtended can be consolidated. I don't think it's worth the complexity and mental overhead unless it significantly reduces performance to send a little extra data to the frontend.

We should always be using the mongoose document type throughout the api + discord-bot. The Praise type should only be used to define the mongoose schema. We should be able to rely on the constraints that mongoose puts on our data models throughout the application. If we're using the Praise type it indicates an potential problem that may introduce bugs.

[kristoferlund]

We should always be using the mongoose document type throughout the api + discord-bot. The Praise type should only be used to define the mongoose schema. We should be able to rely on the constraints that mongoose puts on our data models throughout the application. If we're using the Praise type it indicates an potential problem that may introduce bugs.

Agreed 👍

PraiseDetailsDto, PraiseDto, and PraiseDtoExtended can be consolidated.

Consolidated as in.. brought together into one, right? So, in the end there would be:

• Praise (Not to be used like you said, other than for defining PraiseDocument)
• PraiseDocument
• PraiseDto

In that case yes, sounds like a plan.

I imagine there will be situations where one Collection can be represented by more than one Dto though. A Dto can be defined to suite the special needs for an endpoint - representing a subset of the collection definition, adding some calculated value etc.

[mattyg]

Yes -- and the same for all other data models.

I imagine there will be situations where one Collection can be represented by more than one Dto though. A Dto can be defined to suite the special needs for an endpoint - representing a subset of the collection definition, adding some calculated value etc.

Yes - if there is a significant performance cost to fetching the unneeded data (i.e. enough that it affects the user experience) then we should add another Dto, controller, transformer for the new data type.

Currently there is a lot of logic in the recoil store. And currently all the frontend data is stored in the recoil store. We need to be able to make assumptions that the data in the recoil store reflects the expected data format. Having multiple formats for the same data model in the recoil store adds a lot of unnecessary complexity and boilerplate code that makes it harder to reason about the application. It means having redundant atom's to store another format of the same data, plus any necessary selectors, callbacks, etc.

Or we could store the differently-formatted data in the component store for these cases where the data will not be used in any other context (or when it needs to be re-fetched anyway to be fresh).

[mattyg]

We should always be using the mongoose document type throughout the api + discord-bot. The Praise type should only be used to define the mongoose schema. We should be able to rely on the constraints that mongoose puts on our data models throughout the application. If we're using the Praise type it indicates an potential problem that may introduce bugs.

Agreed +1

Unfortunately I'm realizing this is not going to be possible because we're using aggregate queries. The mongoose 'aggregate' function is just a proxy directly to mongodb -- none of the mongoose constraints are applied and it won't return a mongoose document (so we also can't rely on mongoose virtuals and methods). See https://stackoverflow.com/questions/62559332/mongodb-aggregation-vs-mongoose-virtuals

We would need to migrate away from mongodb+mongoose entirely to get any kind of data constraints enforced throughout the entire application. :face_exhaling: