api/reviews/all API Endpoint to Include Review Status in Response

[kouloumos]

We currently use the api/reviews/all API endpoint to fetch reviews from our system. This endpoint allows us to apply various filters to tailor what is returned. However, there's an important piece of information missing from the responses: the status of each review.

Issue Details
Currently, the status of each review is not included in the API response. As a result, client-side logic has been implemented to determine the status. This approach is not only redundant but also prone to errors, as maintaining this logic across different parts of the application can lead to inconsistencies and bugs.

Proposed Solution
To simplify the process and reduce the potential for errors, we should update the api/reviews/all endpoint to include the status of each review directly in the API response.

[Extheoisah]

In this case, I think it is now time to add a new status column to the DB so that we do not have to compute the status every time the API is called but just fetch it from the DB.

[kouloumos]

In this case, I think it is now time to add a new status column to the DB so that we do not have to compute the status every time the API is called but just fetch it from the DB.

I don't think that this is necessary. I'll try to explain my viewpoint:

Here is an overview of how status works right now.
Currently, there are four fields used to determine the status of a review:

• createdAt: populated when a review is claimed
• submittedAt: populated when a review is submitted
• mergedAt: populated when a review is merged
• archivedAt: populated by the cron-job (runs once a day) which archives the reviews for which 24 hours (default time) have passed since they have been claimed.

As outlined in api/app/utils/review.inference.ts of these fields' presence or absence to determine the review's status.

Your suggestion is to add a new field that directly contains the status. While this would mean the status needs to be derived only once per review, we would still need to maintain the existing fields and the code that computes the status from these four fields. This results in two sources of truth for the review status, plus additional code

What is the computational cost of filtering these reviews each time the api/reviews/all endpoint is called? I believe it is not substantial enough to warrant such a change. @Extheoisah, do you see any other benefits to introducing this new status field?

[Extheoisah]

I thought about this again and you're right about needing to maintain the code that computes the status resulting in additional code. Hence, I'm reevaluating my position for adding the column to db.

As regards the computational cost of filtering these reviews each time the api/reviews/all endpoint is called, we'd have to run a benchmark to see how the API performs when we execute this. Why? Because, for each query, we'd have to use a Promise to run through all the reviews in the db and compute their respective status before sending the data to the client. The time taken for this process to complete will depend on the size of the data being processed and the latency of the db connection. I can't give a definite answer to that unless I write the code for it and benchmark it. However, it shouldn't be significant given this route is only going to be accessed by admins and not by the public.