1
0
mirror of https://github.com/Unleash/unleash.git synced 2024-12-22 19:07:54 +01:00
unleash.unleash/src
Thomas Heartman 0847a395dc
chore: Extract project read model (#7887)
Creates a new project read model exposing data to be used for the UI and
for the insights module.

The model contains two public methods, both based on the project store's
`getProjectsWithCounts`:
- `getProjectsForAdminUi`
- `getProjectsForInsights`

This mirrors the two places where the base query is actually in use
today and adapts the query to those two explicit cases.

The new `getProjectsForAdminUi` method also contains data for last flag
update and last flag metric reported, as required for the new projects
list screen.

Additionally the read model contains a private `getMembersCount` method,
which is also lifted from the project store. This method was only used
in the old `getProjectsWithCounts` method, so I have also removed the
method from the public interface.

This PR does *not* hook up the new read model to anything or delete any
existing uses of the old method.

## Why?

As mentioned in the background, this query is used in two places, both
to get data for the UI (directly or indirectly). This is consistent with
the principles laid out in our [ADR on read vs write
models](https://docs.getunleash.io/contributing/ADRs/back-end/write-model-vs-read-models).

There is an argument to be made, however, that the insights module uses
this as an **internal** read model, but the description of an internal
model ("Internal read models are typically narrowly focused on answering
one question and usually require simple queries compared to external
read models") does not apply here. It's closer to the description of
external read models: "View model will typically join data across a few
DB tables" for display in the UI.

## Discussion points

### What about properties on the schema that are now gone?

The `project-schema`, which is delivered to the UI through the
`getProjects` endpoint (and nowhere else, it seems), describes
properties that will no longer be sent to the front end, including
`defaultStickiness`, `avgTimeToProduction`, and more. Can we just stop
sending them or is that a breaking change?

The schema does not define them as required properties, so in theory,
not sending them isn't breaking any contracts. We can deprecate the
properties and just not populate them anymore.

At least that's my thought on it. I'm open to hearing other views.

### Can we add the properties in fewer lines of code? 

Yes! The [first commit in this PR
(b7534bfa)](b7534bfa07)
adds the two new properties in 8 lines of code.

However, this comes at the cost of diluting the `getProjectsWithCounts`
method further by adding more properties that are not used by the
insights module. That said, that might be a worthwhile tradeoff.

## Background

_(More [details in internal slack
thread](https://unleash-internal.slack.com/archives/C046LV6HH6W/p1723716675436829))_

I noticed that the project store's `getProjectWithCounts` is used in
exactly two places:

1. In the project service method which maps directly to the project
controller (in both OSS and enterprise).
2.  In the insights service in enterprise.

In the case of the controller, that’s the termination point. I’d guess
that when written, the store only served the purpose of showing data to
the UI.

In the event of the insights service, the data is mapped in
getProjectStats.
But I was a little surprised that they were sharing the same query, so I
decided to dig a little deeper to see what we’re actually using and what
we’re not (including the potential new columns). Here’s what I found.

Of the 14 already existing properties, insights use only 7 and the
project list UI uses only 10 (though the schema mentions all 14 (as far
as I could tell from scouring the code base)). Additionally, there’s two
properties that I couldn’t find any evidence of being used by either:
-   default stickiness
-   updatedAt (this is when the project was last updated; not its flags)
2024-08-16 10:52:57 +02:00
..
lib chore: Extract project read model (#7887) 2024-08-16 10:52:57 +02:00
mailtemplates
migrations feat: events table type column index (#7838) 2024-08-12 13:46:27 +03:00
test chore: Extract project read model (#7887) 2024-08-16 10:52:57 +02:00
migrator.ts
server-dev.ts [Gitar] Cleaning up stale flag: improveCreateFlagFlow with value true (#7895) 2024-08-15 15:53:36 +03:00
server.ts