feat(issues): add interactive issues explorer with new pages and navigation#777
Conversation
…gation - Add interactive issues list with advanced filtering and search - Add new Pipelines, Repositories, and Security pages - Update navigation items to include new pages - Enhance issues actions with better data handling
|
@GhanshyamJha05 is attempting to deploy a commit to the codersogs-3057's projects Team on Vercel. A member of the Team first needs to authorize it. |
jakharmonika364
left a comment
jakharmonika364
left a comment
There was a problem hiding this comment.
LGTM, needs second approval before we merge
Siddhartha-singh01
left a comment
Siddhartha-singh01
left a comment
There was a problem hiding this comment.
Review: feat(issues): add interactive issues explorer (#777)
Thanks @GhanshyamJha05 solid UI pass on the issues explorer. First approval is already in from @jakharmonika364 (Needs Second Approval). CI check is green. Vercel red is fork deploy authorization, not app CI.
Summary
This PR:
- Redesigns the issues list (sidebar filters, multi select difficulty/repos, categories, progress widget, drawer polish).
- Extends
getIssuesPagewith multi difficulty, category filters, multi repo filter, andbody_excerpt. - Adds nav entries + placeholder pages for Repositories, Pipelines, and Security.
- Shows contributor progress from completed recommendations count.
UI direction is good and mostly consistent with the MergeShip palette. There are a few correctness / product issues that I would not treat as pure polish.
Verdict
Request changes before second approval merge (or Approve only if the team explicitly accepts shipping placeholder nav + the filter whitelist fix in a fast follow).
Primary blockers from my side:
- Difficulty filter is built by string interpolation into PostgREST
.or()without a whitelist. - Issue cards show a hardcoded “Assigned Mentor” that is not real data.
- Production nav ships three “Under Construction” destinations that look like real product surfaces.
What looks good
Repo ACL preserved (and improved for multi select) in getIssuesPage:
let allowedRepos = repoOptionsRes.data.map((opt) => opt.value);
if (filters.repo) {
const selectedRepos = filters.repo.split(',').filter(Boolean);
if (selectedRepos.length > 0) {
allowedRepos = allowedRepos.filter((r) => selectedRepos.includes(r));
}
}
// ...
.in('repo_full_name', allowedRepos)
|
@Siddhartha-singh01 is it possible to create a sub issue or another issue for these changes as by accident i deleted the folder of changes and cant rretrieve it and am having trouble doing it through the commit alone also tried to get the files by using the commit i made but it is not working for me.... |
|
Hey @GhanshyamJha05 sorry that happened. You do not need a new issue just to recover the code. Your work is still on GitHub on this PR’s branch. The local folder is gone, but the remote branch is not. Recover your changesOn your machine: # 1) Clone your fork (or re-add remote if you still have a clone elsewhere)
git clone https://github.com/GhanshyamJha05/MergeShip.git
cd MergeShip
# 2) Fetch and check out this PR branch
git fetch origin
git checkout feat/issues-explorer-interactive-page-674
# 3) Confirm the files are there
git log -1 --oneline
git show --stat HEAD
If the branch name differs on your fork, open the PR in the browser → green branch name near the top → “Code” / files tab, or:
git fetch origin pull/777/head:pr-777
git checkout pr-777
(That works when run against Coder-s-OG-s/MergeShip as the remote, or after adding upstream.)
You can also download files from the PR Files changed tab if you only need specific files.
About a “sub issue”
• No new issue required to continue this feature keep working on this PR (#777).
• My earlier review still applies on this same PR (request changes). Please address those on this branch when you have the code back:
1. Whitelist difficulty filter values (no raw string into PostgREST .or())
2. Remove hardcoded “Assigned Mentor” / “MS” unless backed by real data
3. Prefer dropping or demoting the Under Construction Repositories / Pipelines / Security items from primary nav
If you want a tracking issue only for those follow-ups, I can open one and link it to #777 but recovery does not depend on that.
Once you’re unblocked and push fixes, comment here and I’ll re-review for second approval.
If you’re still stuck after the steps above, paste the exact error from git fetch / git checkout and I’ll help you through it. |
|
oh my bad i was typing a few words wrong haha will do it in a bit |
|
No worries @GhanshyamJha05 take your time. Once you’ve got the branch back and the review notes addressed, push and ping me here for a re-review. Thanks ! |
|
@Siddhartha-singh01 please check now |
Siddhartha-singh01
left a comment
Siddhartha-singh01
left a comment
There was a problem hiding this comment.
@GhanshyamJha05 checked the new commit thanks for the quick turnaround.
All three review points look addressed:
- Difficulty filter is whitelisted to
E/M/Hbefore the PostgREST.or()path - Hardcoded “Assigned Mentor” / “MS” removed from the issue cards
- Under Construction Repositories / Pipelines / Security removed from primary nav
CI check is green. Vercel is just fork deploy auth.
LGTM ready to merge.
Optional follow-up (non-blocking): the placeholder page files under pipelines/, repositories/, and security/ are still in the tree but unlinked fine to drop in a small cleanup PR later.
Thanks !
|
btw thanks for the help with earlier issue i was using wrong command haha |
|
@GhanshyamJha05 Thanks ! |
3 participants
Summary
This PR adds an interactive issues explorer with enhanced filtering, search, and navigation, along with new supporting pages for Pipelines, Repositories, and Security.
Type of Change
Related Issue
Closes #674
What was changed?
Checklist