fix: update-open-sauced-goals-cache bug

[takanome-dev]

What type of PR is this? (check all applicable)

• 🍕 Feature
• 🐛 Bug Fix
• 📝 Documentation Update
• 🎨 Style
• 🧑‍💻 Code Refactor
• 🔥 Performance Improvements
• ✅ Test
• 🤖 Build
• 🔁 CI
• 📦 Chore (Release)
• ⏩ Revert

Description

This PR fixes the bug in update-open-sauced-goals-cache.js. If open-sauced-goals repo (issue name) do not conform to repo full name conventions, the goal caching action will fail.

Related Tickets & Documents

Fixes #38

Mobile & Desktop Screenshots/Recordings

This is my goals repo. As you can see, since the last 3 days the action keeps failing due to renovatebot Dependency Dashboard issue. But now everything looks good.

Added tests?

• 👍 yes
• 🙅 no, because they aren't needed
• 🙋 no, because I need help

Added to documentation?

• 📜 README.md
• 📓 docs.opensauced.pizza
• 🍕 dev.to/opensauced
• 📕 storybook
• 🙅 no documentation needed

[optional] Are there any post-deployment tasks we need to perform?

[optional] What gif best describes this PR or how it makes you feel?

[bdougie]

Thanks for the PR @takanome-dev. Can you share what you changed in the description. At first glance it looks like an early return but it's all hidden amongst formatting changes.

[takanome-dev]

This is what I added:

async function getRepoGoals(issues){
  return Promise.all(
    issues.map(async issue => {
      // all goal issues follow the "owner/repo" format
      let [owner, name] = issue.title.split("/");
+    if (!owner || !name) return;
    }
  )
}

[mtfoley]

I think the early return will break production, I'd suggest filtering prior to the Promise.all.

@takanome-dev the issue you're seeing here is probably the result of the early return statement:

image from the bug you filed this morning:

affected app code: https://github.com/open-sauced/open-sauced/blob/4a26b94eaa1a1094ee0dd0d7ca76ead50da2acc4/src/components/RepositoryGoals.jsx#L52-L57

null where an object should be: https://github.com/TAKANOME-DEV/open-sauced-goals/blob/main/data.json#L8

[0-vortex]

This is what I added:

async function getRepoGoals(issues){
  return Promise.all(
    issues.map(async issue => {
      // all goal issues follow the "owner/repo" format
      let [owner, name] = issue.title.split("/");
+    if (!owner || !name) return;
    }
  )
}

Please use the integration verification rather than owner plus repo, the actual bug is having issues added by other integrations, it doesn't solve the use case where that integration would add something with a slash in the middle /. Happy to have both solutions in there, however they need some refining.

It would also be great if you limited you pull request to the actual code change rather than reformat the code using prettier. Main motivation for that was that this repo is fairly old and it should only tackle github application integrations for the open-sauced org, tooling and contribution improvements were not added yet. It is unlikely that this repo would take many contributions due to the level of complexity involved in testing :D

[takanome-dev]

Thanks for feedbacks I will work on it ASAP 👍

[takanome-dev]

I made these changes:

async function getRepoGoals(issues){
  return Promise.all(
    issues.map(async issue => {
      // all goal issues follow the "owner/repo" format
      let [owner, name] = issue.title.split("/");
-    if (!owner || !name) return;
    }
  )
}

try {
  stagedIssues = await octokit.rest.issues.listForRepo({
    owner: login,
    repo: "open-sauced-goals",
  });
  console.log("stagedIssues", stagedIssues);
-   repoIssues = await octokit.paginate(stagedIssues);
+  const issues = await octokit.paginate(stagedIssues);
+  // filter issues created by bots
+  repoIssues = issues.filter((repoIssue) => repoIssue.user.type !== "Bot");
} catch (err) {
  console.log(err);
}

[takanome-dev]

Sorry about these commits I had some conflicts to resolve 😓

[0-vortex]

Sorry about these commits I had some conflicts to resolve 😓

No worries, we have squash pull requests for exactly that matter! 🍕