[go: up one dir, main page]

Skip to content

Refactor coupling of `approved?` and `approvals_left`

Description

Currently there are two properties used in approvals:

  • approved? which flags whether a MR is approved or not.
  • approvals_left which tells how many approvals the MR has left to be approved.

Ideally, approved? will always be true if approvals_left is 0, but there is a special situation where this is not the case. If the number of approvers is less than approvals_left, then we consider approved? to be true.

Okay, is this a problem?

From the FE, there is a tight coupling of approved? and approvals_left because if approved? is true we need to treat approvals_left as 0.

Options

Option 1: In the FE when we map the server response, let's overwrite approvals_left to 0 when approved? is true. This keeps the FE from having to pass around two properties, but it does not encapsulate the business logic for other API users 🤷

Option 2: In the BE, cap approvals_required with the number of available approvers.

Proposal

I think Option 2 is the best approach. We'll need to make sure this doesn't mess up the fallback rule.

Edited by Paul Slaughter