Merge branch 'Expensify:main' into fix/sq-crash-window

Author: adhorodyski

.github/actions/javascript/reviewerChecklist/index.js

@@ -11588,6 +11588,10 @@ const pathToReviewerChecklist = 'https://raw.githubusercontent.com/Expensify/App
 const reviewerChecklistContains = '# Reviewer Checklist';
 const issue = github.context.payload.issue?.number ?? github.context.payload.pull_request?.number ?? -1;
 const combinedComments = [];
+// Org members and owners are internal Expensify engineers; external contributors (including C+) are not.
+const INTERNAL_EXPENSIFY_ASSOCIATIONS = new Set(['MEMBER', 'OWNER']);
+// A reviewer's standing is their latest review in one of these states; plain "commented" reviews don't change it.
+const DECISIVE_REVIEW_STATES = new Set(['APPROVED', 'CHANGES_REQUESTED', 'DISMISSED']);
 function getNumberOfItemsFromReviewerChecklist() {
     console.log('Getting the number of items in the reviewer checklist...');
     return new Promise((resolve, reject) => {
@@ -11657,8 +11661,44 @@ function checkIssueForCompletedChecklist(numberOfChecklistItems) {
         core.setFailed("PR Reviewer Checklist is not completely filled out. Please check every box to verify you've thought about the item.");
     });
 }
-getNumberOfItemsFromReviewerChecklist()
-    .then(checkIssueForCompletedChecklist)
+// An approval from an internal Expensify engineer means we've decided this PR doesn't need a C+ checklist, so let the check pass.
+// This workflow re-runs on every pull_request_review event, so we scan the whole review history: once an internal approval
+// stands, a later "commented" or "changes requested" review from anyone must not re-require the checklist.
+async function hasStandingInternalApproval() {
+    const { owner, repo } = github.context.repo;
+    const reviews = await GithubUtils_1.default.paginate(GithubUtils_1.default.octokit.pulls.listReviews, {
+        owner,
+        repo,
+        // eslint-disable-next-line @typescript-eslint/naming-convention
+        pull_number: issue,
+        // eslint-disable-next-line @typescript-eslint/naming-convention
+        per_page: 100,
+    });
+    // GitHub treats a reviewer's latest decisive review as their standing, so keep only that per internal engineer.
+    const latestStateByInternalReviewer = new Map();
+    for (const review of reviews) {
+        const login = review.user?.login;
+        const state = review.state ?? '';
+        if (!login || !INTERNAL_EXPENSIFY_ASSOCIATIONS.has(review.author_association ?? '') || !DECISIVE_REVIEW_STATES.has(state)) {
+            continue;
+        }
+        latestStateByInternalReviewer.set(login, state);
+    }
+    for (const state of latestStateByInternalReviewer.values()) {
+        if (state === 'APPROVED') {
+            return true;
+        }
+    }
+    return false;
+}
+hasStandingInternalApproval()
+    .then((isApproved) => {
+    if (isApproved) {
+        console.log('PR has a standing approval from an internal Expensify engineer, so the reviewer checklist is not required 🎉');
+        return;
+    }
+    return getNumberOfItemsFromReviewerChecklist().then(checkIssueForCompletedChecklist);
+})
     .catch((err) => {
     console.error(err);
     core.setFailed(err);

.github/actions/javascript/reviewerChecklist/reviewerChecklist.ts

@@ -8,6 +8,12 @@ const reviewerChecklistContains = '# Reviewer Checklist';
 const issue: number = github.context.payload.issue?.number ?? github.context.payload.pull_request?.number ?? -1;
 const combinedComments: string[] = [];
 
+// Org members and owners are internal Expensify engineers; external contributors (including C+) are not.
+const INTERNAL_EXPENSIFY_ASSOCIATIONS = new Set(['MEMBER', 'OWNER']);
+
+// A reviewer's standing is their latest review in one of these states; plain "commented" reviews don't change it.
+const DECISIVE_REVIEW_STATES = new Set(['APPROVED', 'CHANGES_REQUESTED', 'DISMISSED']);
+
 function getNumberOfItemsFromReviewerChecklist() {
     console.log('Getting the number of items in the reviewer checklist...');
     return new Promise<number>((resolve, reject) => {
@@ -87,8 +93,47 @@ function checkIssueForCompletedChecklist(numberOfChecklistItems: number) {
         });
 }
 
-getNumberOfItemsFromReviewerChecklist()
-    .then(checkIssueForCompletedChecklist)
+// An approval from an internal Expensify engineer means we've decided this PR doesn't need a C+ checklist, so let the check pass.
+// This workflow re-runs on every pull_request_review event, so we scan the whole review history: once an internal approval
+// stands, a later "commented" or "changes requested" review from anyone must not re-require the checklist.
+async function hasStandingInternalApproval(): Promise<boolean> {
+    const {owner, repo} = github.context.repo;
+    const reviews = await GitHubUtils.paginate(GitHubUtils.octokit.pulls.listReviews, {
+        owner,
+        repo,
+        // eslint-disable-next-line @typescript-eslint/naming-convention
+        pull_number: issue,
+        // eslint-disable-next-line @typescript-eslint/naming-convention
+        per_page: 100,
+    });
+
+    // GitHub treats a reviewer's latest decisive review as their standing, so keep only that per internal engineer.
+    const latestStateByInternalReviewer = new Map<string, string>();
+    for (const review of reviews) {
+        const login = review.user?.login;
+        const state = review.state ?? '';
+        if (!login || !INTERNAL_EXPENSIFY_ASSOCIATIONS.has(review.author_association ?? '') || !DECISIVE_REVIEW_STATES.has(state)) {
+            continue;
+        }
+        latestStateByInternalReviewer.set(login, state);
+    }
+
+    for (const state of latestStateByInternalReviewer.values()) {
+        if (state === 'APPROVED') {
+            return true;
+        }
+    }
+    return false;
+}
+
+hasStandingInternalApproval()
+    .then((isApproved) => {
+        if (isApproved) {
+            console.log('PR has a standing approval from an internal Expensify engineer, so the reviewer checklist is not required 🎉');
+            return;
+        }
+        return getNumberOfItemsFromReviewerChecklist().then(checkIssueForCompletedChecklist);
+    })
     .catch((err: string | Error) => {
         console.error(err);
         core.setFailed(err);

.github/workflows/authorChecklist.yml

@@ -31,7 +31,11 @@ jobs:
           OS_BOTIFY_TOKEN: ${{ secrets.OS_BOTIFY_TOKEN }}
 
       - name: authorChecklist.ts
-        if: steps.gate.outputs.IS_AUTHORIZED == 'true'
+        # Internal Expensify engineers (org members/owners) skip the author checklist; external contributors still fill it out.
+        if: |
+          steps.gate.outputs.IS_AUTHORIZED == 'true'
+          && github.event.pull_request.author_association != 'MEMBER'
+          && github.event.pull_request.author_association != 'OWNER'
         uses: ./.github/actions/javascript/authorChecklist
         with:
           GITHUB_TOKEN: ${{ github.token }}

Mobile-Expensify

@@ -111,8 +111,8 @@ android {
         minSdkVersion rootProject.ext.minSdkVersion
         targetSdkVersion rootProject.ext.targetSdkVersion
         multiDexEnabled rootProject.ext.multiDexEnabled
-        versionCode 1009039908
-        versionName "9.3.99-8"
+        versionCode 1009040000
+        versionName "9.4.0-0"
         // Supported language variants must be declared here to avoid from being removed during the compilation.
         // This also helps us to not include unnecessary language variants in the APK.
         resConfigs "en", "es"

android/app/build.gradle

@@ -336,7 +336,6 @@
 "../../src/libs/actions/IOU/SendMoney.ts"	"no-restricted-syntax"	1
 "../../src/libs/actions/IOU/Split.ts"	"@typescript-eslint/no-deprecated/InteractionManager.runAfterInteractions"	4
 "../../src/libs/actions/IOU/Split.ts"	"@typescript-eslint/no-deprecated/Localize.translateLocal"	2
-"../../src/libs/actions/IOU/Split.ts"	"@typescript-eslint/no-deprecated/getMoneyRequestPolicyTags"	1
 "../../src/libs/actions/IOU/Split.ts"	"no-restricted-syntax"	3
 "../../src/libs/actions/IOU/SplitTransactionUpdate.ts"	"@typescript-eslint/no-deprecated/InteractionManager.runAfterInteractions"	1
 "../../src/libs/actions/IOU/SplitTransactionUpdate.ts"	"@typescript-eslint/no-deprecated/getMoneyRequestPolicyTags"	1

assets/images/moon.svg

@@ -56,7 +56,7 @@ Below are some examples to illustrate the writing style that covers this:
 - Bug: A blank page is shown for an archived room with a message in it
 - Proposed Test Steps:
   - Create a workspace if you don't have any
-  - Go to members page and remove the other admin (Expensify setup specialist)
+  - Go to members page and remove the other admin (Expensify account executive)
   - Search the announce room and send a message
   - Pin the room and delete the workspace
   - Wait for a few seconds (Reload if the chat is still visible)