Merge pull request #27532 from software-mansion-labs/Sebryu-DynamicCh…

…ecklistPOC

[POC] Dynamic checklist

.github/PULL_REQUEST_TEMPLATE.md

@@ -94,17 +94,6 @@ This is a checklist for PR authors. Please make sure to complete all tasks and c
 - [ ] I verified all code is DRY (the PR doesn't include any logic written more than once, with the exception of tests)
 - [ ] I verified any variables that can be defined as constants (ie. in CONST.js or at the top of the file that uses the constant) are defined as such
 - [ ] I verified that if a function's arguments changed that all usages have also been updated correctly
-- [ ] If a new component is created I verified that:
-    - [ ] A similar component doesn't exist in the codebase
-    - [ ] All props are defined accurately and each prop has a `/** comment above it */`
-    - [ ] The file is named correctly
-    - [ ] The component has a clear name that is non-ambiguous and the purpose of the component can be inferred from the name alone
-    - [ ] The only data being stored in the state is data necessary for rendering and nothing else
-    - [ ] If we are not using the full Onyx data that we loaded, I've added the proper selector in order to ensure the component only re-renders when the data it is using changes
-    - [ ] For Class Components, any internal methods passed to components event handlers are bound to `this` properly so there are no scoping issues (i.e. for `onClick={this.submit}` the method `this.submit` should be bound to `this` in the constructor)
-    - [ ] Any internal methods bound to `this` are necessary to be bound (i.e. avoid `this.submit = this.submit.bind(this);` if `this.submit` is never passed to a component event handler like `onClick`)
-    - [ ] All JSX used for rendering exists in the render method
-    - [ ] The component has the minimum amount of code necessary for its purpose, and it is broken down into smaller components in order to separate concerns and functions
 - [ ] If any new file was added I verified that:
     - [ ] The file has a description of what it does and/or why is needed at the top of the file if the code is not self explanatory
 - [ ] If a new CSS style is added I verified that:
@@ -116,7 +105,6 @@ This is a checklist for PR authors. Please make sure to complete all tasks and c
 - [ ] If the PR modifies a component or page that can be accessed by a direct deeplink, I verified that the code functions as expected when the deeplink is used - from a logged in and logged out account.
 - [ ] If a new page is added, I verified it's using the `ScrollView` component to make it scrollable when more elements are added to the page.
 - [ ] If the `main` branch was merged into this PR after a review, I tested again and verified the outcome was still expected according to the `Test` steps.
-- [ ] I have checked off every checkbox in the PR author checklist, including those that don't apply to this PR.
 
 ### Screenshots/Videos
 <details>

.github/actions/javascript/authorChecklist/authorChecklist.ts

@@ -0,0 +1,158 @@
+import core from '@actions/core';
+import github from '@actions/github';
+import escapeRegExp from 'lodash/escapeRegExp';
+import CONST from '../../../libs/CONST';
+import GithubUtils from '../../../libs/GithubUtils';
+import newComponentCategory from './categories/newComponentCategory';
+
+const pathToAuthorChecklist = `https://raw.githubusercontent.com/${CONST.GITHUB_OWNER}/${CONST.APP_REPO}/main/.github/PULL_REQUEST_TEMPLATE.md`;
+const checklistStartsWith = '### PR Author Checklist';
+const checklistEndsWith = '\r\n### Screenshots/Videos';
+
+const prNumber = github.context.payload.pull_request?.number;
+
+const CHECKLIST_CATEGORIES = {
+    NEW_COMPONENT: newComponentCategory,
+};
+
+/**
+ * Look at the contents of the pull request, and determine which checklist categories apply.
+ */
+async function getChecklistCategoriesForPullRequest(): Promise<Set<string>> {
+    const checks = new Set<string>();
+    const changedFiles = await GithubUtils.paginate(GithubUtils.octokit.pulls.listFiles, {
+        owner: CONST.GITHUB_OWNER,
+        repo: CONST.APP_REPO,
+        // eslint-disable-next-line @typescript-eslint/naming-convention
+        pull_number: prNumber,
+        // eslint-disable-next-line @typescript-eslint/naming-convention
+        per_page: 100,
+    });
+    const possibleCategories = await Promise.all(
+        Object.values(CHECKLIST_CATEGORIES).map(async (category) => ({
+            items: category.items,
+            doesCategoryApply: await category.detect(changedFiles),
+        })),
+    );
+    for (const category of possibleCategories) {
+        if (category.doesCategoryApply) {
+            for (const item of category.items) {
+                checks.add(item);
+            }
+        }
+    }
+    return checks;
+}
+
+function partitionWithChecklist(body: string): string[] {
+    const [contentBeforeChecklist, contentAfterStartOfChecklist] = body.split(checklistStartsWith);
+    const [checklistContent, contentAfterChecklist] = contentAfterStartOfChecklist.split(checklistEndsWith);
+    return [contentBeforeChecklist, checklistContent, contentAfterChecklist];
+}
+
+async function getNumberOfItemsFromAuthorChecklist(): Promise<number> {
+    const response = await fetch(pathToAuthorChecklist);
+    const fileContents = await response.text();
+    const checklist = partitionWithChecklist(fileContents)[1];
+    const numberOfChecklistItems = (checklist.match(/\[ \]/g) ?? []).length;
+    return numberOfChecklistItems;
+}
+
+function checkPRForCompletedChecklist(expectedNumberOfChecklistItems: number, checklist: string) {
+    const numberOfFinishedChecklistItems = (checklist.match(/- \[x\]/gi) ?? []).length;
+    const numberOfUnfinishedChecklistItems = (checklist.match(/- \[ \]/g) ?? []).length;
+
+    const minCompletedItems = expectedNumberOfChecklistItems - 2;
+
+    console.log(`You completed ${numberOfFinishedChecklistItems} out of ${expectedNumberOfChecklistItems} checklist items with ${numberOfUnfinishedChecklistItems} unfinished items`);
+
+    if (numberOfFinishedChecklistItems >= minCompletedItems && numberOfUnfinishedChecklistItems === 0) {
+        console.log('PR Author checklist is complete 🎉');
+        return;
+    }
+
+    console.log(`Make sure you are using the most up to date checklist found here: ${pathToAuthorChecklist}`);
+    core.setFailed("PR Author Checklist is not completely filled out. Please check every box to verify you've thought about the item.");
+}
+
+async function generateDynamicChecksAndCheckForCompletion() {
+    // Generate dynamic checks
+    const dynamicChecks = await getChecklistCategoriesForPullRequest();
+    let isPassing = true;
+    let didChecklistChange = false;
+
+    const body = github.context.payload.pull_request?.body ?? '';
+
+    // eslint-disable-next-line prefer-const
+    let [contentBeforeChecklist, checklist, contentAfterChecklist] = partitionWithChecklist(body);
+
+    for (const check of dynamicChecks) {
+        // Check if it's already in the PR body, capturing the whether or not it's already checked
+        const regex = new RegExp(`- \\[([ x])] ${escapeRegExp(check)}`);
+        const match = regex.exec(checklist);
+        if (!match) {
+            // Add it to the PR body
+            isPassing = false;
+            checklist += `- [ ] ${check}\r\n`;
+            didChecklistChange = true;
+        } else {
+            const isChecked = match[1] === 'x';
+            if (!isChecked) {
+                isPassing = false;
+            }
+        }
+    }
+
+    // Check if some dynamic check was added with previous commit, but is not relevant anymore
+    const allChecks = Object.values(CHECKLIST_CATEGORIES).reduce((acc: string[], category) => acc.concat(category.items), []);
+
+    for (const check of allChecks) {
+        if (!dynamicChecks.has(check)) {
+            const regex = new RegExp(`- \\[([ x])] ${escapeRegExp(check)}\r\n`);
+            const match = regex.exec(checklist);
+            if (match) {
+                // Remove it from the PR body
+                checklist = checklist.replace(match[0], '');
+                didChecklistChange = true;
+            }
+        }
+    }
+
+    // Put the PR body back together, need to add the markers back in
+    const newBody = contentBeforeChecklist + checklistStartsWith + checklist + checklistEndsWith + contentAfterChecklist;
+
+    // Update the PR body
+    if (didChecklistChange) {
+        await GithubUtils.octokit.pulls.update({
+            owner: CONST.GITHUB_OWNER,
+            repo: CONST.APP_REPO,
+            // eslint-disable-next-line @typescript-eslint/naming-convention
+            pull_number: prNumber,
+            body: newBody,
+        });
+        console.log('Updated PR checklist');
+    }
+
+    if (!isPassing) {
+        const err = new Error("New checks were added into checklist. Please check every box to verify you've thought about the item.");
+        console.error(err);
+        core.setFailed(err);
+    }
+
+    // check for completion
+    try {
+        const numberOfItems = await getNumberOfItemsFromAuthorChecklist();
+        checkPRForCompletedChecklist(numberOfItems, checklist);
+    } catch (error) {
+        console.error(error);
+        if (error instanceof Error) {
+            core.setFailed(error.message);
+        }
+    }
+}
+
+if (require.main === module) {
+    generateDynamicChecksAndCheckForCompletion();
+}
+
+export default generateDynamicChecksAndCheckForCompletion;

.github/actions/javascript/authorChecklist/categories/Category.ts

@@ -0,0 +1,6 @@
+type Category = {
+    detect: (changedFiles: Array<{filename: string; status: string}>) => Promise<boolean>;
+    items: string[];
+};
+
+export default Category;

.github/actions/javascript/authorChecklist/categories/index.ts

@@ -0,0 +1,6 @@
+import Category from './Category';
+import newComponent from './newComponentCategory';
+
+const categories: Category[] = [newComponent];
+
+export default categories;

.github/actions/javascript/authorChecklist/categories/newComponentCategory.ts

@@ -0,0 +1,110 @@
+import github from '@actions/github';
+import {parse} from '@babel/parser';
+import traverse from '@babel/traverse';
+import CONST from '../../../../libs/CONST';
+import GithubUtils from '../../../../libs/GithubUtils';
+import promiseSome from '../../../../libs/promiseSome';
+import Category from './Category';
+
+type SuperClassType = {superClass: {name?: string; object: {name: string}; property: {name: string}} | null; name: string};
+
+const items = [
+    "I verified that similar component doesn't exist in the codebase",
+    'I verified that all props are defined accurately and each prop has a `/** comment above it */`',
+    'I verified that each file is named correctly',
+    'I verified that each component has a clear name that is non-ambiguous and the purpose of the component can be inferred from the name alone',
+    'I verified that the only data being stored in component state is data necessary for rendering and nothing else',
+    "In component if we are not using the full Onyx data that we loaded, I've added the proper selector in order to ensure the component only re-renders when the data it is using changes",
+    'For Class Components, any internal methods passed to components event handlers are bound to `this` properly so there are no scoping issues (i.e. for `onClick={this.submit}` the method `this.submit` should be bound to `this` in the constructor)',
+    'I verified that component internal methods bound to `this` are necessary to be bound (i.e. avoid `this.submit = this.submit.bind(this);` if `this.submit` is never passed to a component event handler like `onClick`)',
+    'I verified that all JSX used for rendering exists in the render method',
+    'I verified that each component has the minimum amount of code necessary for its purpose, and it is broken down into smaller components in order to separate concerns and functions',
+];
+
+function isComponentOrPureComponent(name?: string) {
+    return name === 'Component' || name === 'PureComponent';
+}
+
+function detectReactComponent(code: string, filename: string): boolean | undefined {
+    if (!code) {
+        console.error('failed to get code from a filename', code, filename);
+        return;
+    }
+    const ast = parse(code, {
+        sourceType: 'module',
+        plugins: ['jsx', 'typescript'], // enable jsx plugin
+    });
+
+    let isReactComponent = false;
+
+    traverse(ast, {
+        enter(path) {
+            if (isReactComponent) {
+                return;
+            }
+            if (path.isFunctionDeclaration() || path.isArrowFunctionExpression() || path.isFunctionExpression()) {
+                path.traverse({
+                    // eslint-disable-next-line @typescript-eslint/naming-convention
+                    JSXElement() {
+                        isReactComponent = true;
+                        path.stop();
+                    },
+                });
+            }
+        },
+        // eslint-disable-next-line @typescript-eslint/naming-convention
+        ClassDeclaration(path) {
+            const {superClass} = path.node as unknown as SuperClassType;
+            if (
+                superClass &&
+                ((superClass.object && superClass.object.name === 'React' && isComponentOrPureComponent(superClass.property.name)) || isComponentOrPureComponent(superClass.name))
+            ) {
+                isReactComponent = true;
+                path.stop();
+            }
+        },
+    });
+
+    return isReactComponent;
+}
+
+function nodeBase64ToUtf8(data: string) {
+    return Buffer.from(data, 'base64').toString('utf-8');
+}
+
+async function detectReactComponentInFile(filename: string): Promise<boolean | undefined> {
+    const params = {
+        owner: CONST.GITHUB_OWNER,
+        repo: CONST.APP_REPO,
+        path: filename,
+        ref: github.context.payload.pull_request?.head.ref,
+    };
+    try {
+        const {data} = await GithubUtils.octokit.repos.getContent(params);
+        const content = 'content' in data ? nodeBase64ToUtf8(data.content || '') : data;
+        return detectReactComponent(content, filename);
+    } catch (error) {
+        console.error('An unknown error occurred with the GitHub API: ', error, params);
+    }
+}
+
+async function detect(changedFiles: Array<{filename: string; status: string}>): Promise<boolean> {
+    const filteredFiles = changedFiles.filter(({filename, status}) => status === 'added' && (filename.endsWith('.js') || filename.endsWith('.ts') || filename.endsWith('.tsx')));
+    try {
+        await promiseSome(
+            filteredFiles.map(({filename}) => detectReactComponentInFile(filename)),
+            (result) => !!result,
+        );
+        return true;
+    } catch (err) {
+        return false;
+    }
+}
+
+const newComponentCategory: Category = {
+    detect,
+    items,
+};
+
+export default newComponentCategory;
+export {detectReactComponent};