Is it necessary to compare before and after CHANGES_REQUESTED and APPROVED in getApproveTime?

[kawamura-lvgs]

Hello, I have a question regarding the implementation of getApproveTime.

In the implementation of getApproveTime, it looks like this: https://github.com/AlexSim93/pull-request-analytics-action/blob/master/src/converters/utils/calculations/getApproveTime.ts

import { makeComplexRequest } from "../../../requests";

export const getApproveTime = (
  reviews: Awaited<ReturnType<typeof makeComplexRequest>>["reviews"][number]
) => {
  const reviewChangesRequested = reviews?.reduce((acc, review) => {
    if (review.state === "CHANGES_REQUESTED") {
      const login = review.user?.login || "invalid";
      return { ...acc, [login]: true };
    }
    return acc;
  }, {}) as Record<string, boolean> | undefined;
  const reviewApproved = reviews?.reduce((acc, review) => {
    if (review.state === "APPROVED") {
      const login = review.user?.login || "invalid";
      return { ...acc, [login]: review.submitted_at };
    }
    return acc;
  }, {}) as Record<string, string> | undefined;
  const usersRequestedChanges = reviewChangesRequested
    ? Object.keys(reviewChangesRequested)
    : [];
  if (usersRequestedChanges.length === 0) {
    return reviews?.find((review) => review.state === "APPROVED")?.submitted_at;
  }

  const approveEntry = Object.entries(reviewApproved || {}).find(([user]) => {
    if (reviewChangesRequested?.[user]) {
      reviewChangesRequested[user] = false;

      return !Object.values(reviewChangesRequested).some(
        (value) => value === true
      );
    }
    return false;
  });

  return approveEntry?.[1];
};

In the current implementation, there is no comparison before and after CHANGES_REQUESTED and APPROVED.

Therefore, even if CHANGES_REQUESTED is performed after APPROVED, the value of approveEntry can still be obtained. The data I want to obtain should ideally be in the state where all reviews are APPROVED, but this implementation does not achieve that.

Was this implementation intentional? Your feedback would be appreciated.

[AlexSim93]

Hello, probably there is a bug. I wanted to implement it as you said. I will check and fix it.

[AlexSim93]

If you have multiple reviewers(one just commented and second approved) then this PR will be considered as approved. Once Changes requested are dismissed it is considered as changes are not required.

[kawamura-lvgs]

Thank you!