[codex] Add ERP project tracking views#291
Conversation
|
Caution Review failedThe pull request is closed. ℹ️ Recent review info⚙️ Run configurationConfiguration used: defaults Review profile: CHILL Plan: Pro Run ID: 📒 Files selected for processing (25)
📝 WalkthroughWalkthroughThis PR adds a complete ERPNext project integration feature: ERPNext HTTP client for API communication, local PostgreSQL cache with synchronization job, roster-based visibility filtering for dashboard access, wiki document parsing with fuzzy project matching, React admin dashboard components with full CRUD operations, Discord bot command for read-only project lookups, and comprehensive tests. ChangesERPNext Projects Feature
Estimated code review effort🎯 4 (Complex) | ⏱️ ~75 minutes Possibly related PRs
🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches📝 Generate docstrings
🧪 Generate unit tests (beta)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
![chatgpt-codex-connector[bot]](https://avatars.githubusercontent.com/in/1144995?s=60&v=4){kind=small}
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 1c2b2cfc9b
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
There was a problem hiding this comment.
Pull request overview
This PR adds ERPNext-backed project tracking across the stack by introducing a shared ERPNext client, Postgres-backed project/roster cache tables + worker sync job, and new read-only project views in both the operations dashboard and Discord.
Changes:
- Add ERPNext API client + worker job to sync open ERPNext projects and their roster into local cache tables.
- Add shared project-cache querying helpers (dashboard listing, roster membership checks, wiki table parsing + fuzzy match preview).
- Add dashboard + Discord
/projectssurfaces with roster-aware visibility filtering and new permissions.
Reviewed changes
Copilot reviewed 18 out of 21 changed files in this pull request and generated 3 comments.
Show a summary per file
| File | Description |
|---|---|
| tests/unit/test_projects.py | Adds unit coverage for ERPNext payload mapping, wiki table parsing/matching, and Discord viewer email resolution. |
| tests/unit/test_backend_api.py | Adds API tests for project read permission and roster-filtered dashboard projects responses. |
| packages/shared/src/five08/settings.py | Adds ERPNext settings (base URL, API key, timeout). |
| packages/shared/src/five08/projects.py | Introduces project cache upsert/query logic, roster helpers, and wiki fuzzy-match utilities. |
| packages/shared/src/five08/clients/erpnext.py | Adds a small authenticated ERPNext client wrapper with error handling. |
| packages/shared/src/five08/clients/init.py | Exposes the new erpnext client module. |
| apps/worker/src/five08/worker/migrations/versions/20260519_0100_create_project_cache_tables.py | Creates projects, project_external_ids, and project_roster_members tables + triggers/indexes. |
| apps/worker/src/five08/worker/jobs.py | Registers a new worker job to sync projects from ERPNext. |
| apps/worker/src/five08/worker/erpnext_project_sync.py | Implements the ERPNext → cache sync processor. |
| apps/discord_bot/src/five08/discord_bot/cogs/projects.py | Adds /projects command backed by the project cache with roster-based visibility. |
| apps/api/src/five08/backend/static/dashboard/index.html | Updates dashboard asset references (rebuilt frontend). |
| apps/api/src/five08/backend/static/dashboard/assets/index-DfimwzZZ.css | Adds new built CSS bundle. |
| apps/api/src/five08/backend/static/dashboard/assets/index-Cdw3ivTK.css | Removes old built CSS bundle. |
| apps/api/src/five08/backend/static/dashboard/.vite/manifest.json | Updates Vite manifest to point at new built assets. |
| apps/api/src/five08/backend/dashboard.py | Adds /dashboard/projects route to the dashboard route list. |
| apps/api/src/five08/backend/auth.py | Adds project permissions and includes them in permission sets. |
| apps/api/src/five08/backend/api.py | Adds project dashboard APIs, wiki match preview API, and ERPNext sync enqueue endpoint. |
| apps/admin_dashboard/src/main.tsx | Adds Projects view UI, loading/sync actions, and wiki match preview display. |
| .env.example | Documents ERPNext env vars required for project sync and /projects. |
Files not reviewed (1)
- apps/api/src/five08/backend/static/dashboard/assets/index-DfimwzZZ.css: Language not supported
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 6
🧹 Nitpick comments (2)
tests/unit/test_backend_api.py (1)
2606-2640: ⚡ Quick winAdd assertions to verify the steering committee's unrestricted access.
The test patches
list_dashboard_projectsbut doesn't verify it was called with the correct parameters. Unlike the member test (lines 2596-2603), this test doesn't assert thatinclude_all=Trueor verify theviewer_emailsparameter, which reduces confidence that steering users actually bypass roster filtering.✅ Suggested assertions to add
with ( patch( "five08.backend.api._current_session", new_callable=AsyncMock, return_value=("session-1", session), ), - patch("five08.backend.api.list_dashboard_projects", return_value=[]), + patch("five08.backend.api.list_dashboard_projects", return_value=[]) as mock_projects, patch("five08.backend.api.project_cache_summary", return_value=summary), ): response = client.get("/dashboard/api/projects") assert response.status_code == 200 assert response.json() == {"projects": [], "summary": summary} + mock_projects.assert_called_once_with( + api.settings, + query=None, + status=None, + viewer_emails=None, + include_all=True, + limit=100, + )🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@tests/unit/test_backend_api.py` around lines 2606 - 2640, Update the test_dashboard_projects_allows_steering_to_see_all_projects test to assert that list_dashboard_projects was invoked with the steering user's unrestricted parameters: after calling client.get("/dashboard/api/projects"), retrieve the mock for list_dashboard_projects and assert it was called once with include_all=True and the expected viewer_emails (e.g., ["steering@508.dev"] or None depending on how the code passes viewer_emails) and any other positional args the real call uses; reference the patched symbol list_dashboard_projects and the session fixture (session with email "steering@508.dev") to determine the exact expected viewer_emails value and include that assertion alongside the existing response.status_code/JSON checks.apps/worker/src/five08/worker/erpnext_project_sync.py (1)
46-61: ⚡ Quick winLog conversion failures for better observability.
When
erpnext_project_to_input()returnsNone(line 49), the project is silently added tofailed_project_idswithout logging the reason. This makes it difficult to diagnose which projects failed and why during sync operations.📋 Add logging for conversion failures
try: detail = self.client.get_project(project_id) payload = erpnext_project_to_input(detail) if payload is None: + logger.warning( + "erpnext_project_to_input returned None for project id=%s; " + "check payload structure or required fields", + project_id, + ) failed_project_ids.append(project_id) continue🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@apps/worker/src/five08/worker/erpnext_project_sync.py` around lines 46 - 61, When erpnext_project_to_input(detail) returns None the code currently just appends project_id to failed_project_ids; modify the None branch in the loop (where erpnext_project_to_input is called) to log a warning or info with context before appending: include project_id, any identifying fields from detail (e.g., detail.get("name") or detail.get("id")), and a short message that conversion returned None so operators can see which project failed conversion; keep the existing exception handler using logger.warning for other failures unchanged.
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@apps/api/src/five08/backend/api.py`:
- Around line 802-806: The permission and project-visibility checks call
blocking DB functions on the event loop; update _session_dashboard_permissions
to call _session_can_view_any_project via asyncio.to_thread(...) (or refactor to
async), and in dashboard_projects_handler and
dashboard_project_wiki_matches_handler call _dashboard_project_viewer_emails
using asyncio.to_thread(...) as well (or convert those helper functions to async
DB calls); ensure returned values are awaited and used the same way so the
permission resolution remains correct but no blocking psycopg queries run on the
main event loop.
In `@apps/worker/src/five08/worker/erpnext_project_sync.py`:
- Around line 35-39: The call to self.client.list_projects(status="Open",
limit=500) can silently truncate results; update the sync in ERPNextProjectSync
(the block around list_projects and the ERPNextAPIError except) to detect
potential truncation by checking the returned project_refs length against the
requested limit and emit a warning via logger.warn/logger.warning when they are
equal (e.g., "Got 500 projects, results may be truncated - enable pagination");
keep the existing exception handling for ERPNextAPIError, and as a follow-up
note add TODO or implement proper pagination using offset/cursor if the client
supports it.
In
`@apps/worker/src/five08/worker/migrations/versions/20260519_0100_create_project_cache_tables.py`:
- Line 27: Add a CHECK constraint to enforce percent_complete is NULL or between
0 and 100: after the op.create_table(...) that defines the percent_complete
column, execute an ALTER TABLE to ADD CONSTRAINT check_percent_complete_range
CHECK (percent_complete IS NULL OR (percent_complete >= 0 AND percent_complete
<= 100)); in downgrade() before dropping the table execute an ALTER TABLE to
DROP CONSTRAINT IF EXISTS check_percent_complete_range; reference the
percent_complete column, the op.create_table call that creates the projects
table, and the downgrade() function when making these changes.
In `@packages/shared/src/five08/projects.py`:
- Around line 149-152: The project id resolution currently happens in
_resolve_or_create_project_id outside the transaction that writes project and
roster, which allows duplicate project rows under concurrency; change
upsert_project so the resolve/create and all subsequent writes occur inside a
single DB transaction (or use the same DB connection/transaction), e.g. call a
variant of _resolve_or_create_project_id that accepts the transaction/connection
or inline the id resolution using INSERT ... ON CONFLICT ... RETURNING id (or
SELECT ... FOR UPDATE) within the same transaction, then perform the projects,
project_external_ids and roster upserts before committing; apply the same
pattern for the other affected block (lines referenced 266-323) so id resolution
and writes are atomic.
- Line 668: The regex in the re.sub call currently uses a double-escaped
sequence (re.sub(r"https?://\\S+", " ", text)) which matches a literal backslash
and "S" instead of non-whitespace characters; update that call to use a raw
regex with a single backslash (e.g., re.sub(r"https?://\S+", " ", text)) so URLs
are correctly stripped from the variable text; locate the re.sub(...) invocation
in the function surrounding the variable text and replace the pattern
accordingly, then run relevant fuzzy-match/cleanup tests.
- Around line 354-359: The membership filter builds normalized_viewer_emails
from viewer_emails but calls email.casefold() without trimming, so
leading/trailing whitespace can prevent matches; change the comprehension to
first call text_or_none(email) into a temp (e.g., t = text_or_none(email)), skip
if None, then use t.strip().casefold() (or strip before casefold) when adding to
normalized_viewer_emails so stored values are trimmed and casefolded; update the
comprehension around normalized_viewer_emails and keep references to
viewer_emails and text_or_none.
---
Nitpick comments:
In `@apps/worker/src/five08/worker/erpnext_project_sync.py`:
- Around line 46-61: When erpnext_project_to_input(detail) returns None the code
currently just appends project_id to failed_project_ids; modify the None branch
in the loop (where erpnext_project_to_input is called) to log a warning or info
with context before appending: include project_id, any identifying fields from
detail (e.g., detail.get("name") or detail.get("id")), and a short message that
conversion returned None so operators can see which project failed conversion;
keep the existing exception handler using logger.warning for other failures
unchanged.
In `@tests/unit/test_backend_api.py`:
- Around line 2606-2640: Update the
test_dashboard_projects_allows_steering_to_see_all_projects test to assert that
list_dashboard_projects was invoked with the steering user's unrestricted
parameters: after calling client.get("/dashboard/api/projects"), retrieve the
mock for list_dashboard_projects and assert it was called once with
include_all=True and the expected viewer_emails (e.g., ["steering@508.dev"] or
None depending on how the code passes viewer_emails) and any other positional
args the real call uses; reference the patched symbol list_dashboard_projects
and the session fixture (session with email "steering@508.dev") to determine the
exact expected viewer_emails value and include that assertion alongside the
existing response.status_code/JSON checks.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: defaults
Review profile: CHILL
Plan: Pro
Run ID: b107d7db-3b97-4d1b-b831-c3a39fd8a0ed
📒 Files selected for processing (21)
.env.exampleapps/admin_dashboard/src/main.tsxapps/api/src/five08/backend/api.pyapps/api/src/five08/backend/auth.pyapps/api/src/five08/backend/dashboard.pyapps/api/src/five08/backend/static/dashboard/.vite/manifest.jsonapps/api/src/five08/backend/static/dashboard/assets/index-CHUx-bdo.jsapps/api/src/five08/backend/static/dashboard/assets/index-Cdw3ivTK.cssapps/api/src/five08/backend/static/dashboard/assets/index-DfimwzZZ.cssapps/api/src/five08/backend/static/dashboard/assets/index-k4LODCF_.jsapps/api/src/five08/backend/static/dashboard/index.htmlapps/discord_bot/src/five08/discord_bot/cogs/projects.pyapps/worker/src/five08/worker/erpnext_project_sync.pyapps/worker/src/five08/worker/jobs.pyapps/worker/src/five08/worker/migrations/versions/20260519_0100_create_project_cache_tables.pypackages/shared/src/five08/clients/__init__.pypackages/shared/src/five08/clients/erpnext.pypackages/shared/src/five08/projects.pypackages/shared/src/five08/settings.pytests/unit/test_backend_api.pytests/unit/test_projects.py
💤 Files with no reviewable changes (1)
- apps/api/src/five08/backend/static/dashboard/assets/index-Cdw3ivTK.css
michaelmwu
force-pushed
the
michaelmwu/erp-project-status-view
branch
from
1c2b2cf to
c3b51ae
Compare
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: c3b51aec04
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
| if not include_all: | ||
| preview["wiki_rows"] = [] |
There was a problem hiding this comment.
For non-Steering users, this endpoint clears only preview["wiki_rows"], but wiki_project_match_preview() has already matched each visible project against the full Outline document and includes the selected wiki row under matches[].best_match.row. A roster-limited project member can therefore call /dashboard/api/projects/wiki-matches and receive DRI/member/client/status data from unrelated wiki rows whenever those rows are the best fuzzy match, despite the attempted top-level redaction.
Useful? React with 👍 / 👎.
| project_refs = self.client.list_projects( | ||
| status="Open", | ||
| limit=project_limit, | ||
| ) |
There was a problem hiding this comment.
When ERPNext has more than 500 open projects, this sync requests only one page and never follows up with limit_start/additional pages, so projects beyond the first 500 are never inserted or refreshed for the dashboard and /projects command. The later likely_truncated guard only avoids marking missing projects stale; it does not populate the omitted open projects.
Useful? React with 👍 / 👎.
michaelmwu
force-pushed
the
michaelmwu/erp-project-status-view
branch
from
c3b51ae to
ff7a901
Compare
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: ff7a9016f1
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
| {[project.actual_start_date, project.actual_end_date] | ||
| .filter(Boolean) | ||
| .map((value) => formatDate(value)) |
There was a problem hiding this comment.
When ERPNext sends actual_start_date/actual_end_date as date-only values, the backend serializes them as YYYY-MM-DD, and passing those through formatDate() constructs a JS Date at UTC midnight. In browsers west of UTC, this renders as the prior local day with a time (for example 2026-01-02 can display as Jan 1 evening), so project timelines are shown incorrectly for many US users; use a date-only formatter for these fields.
Useful? React with 👍 / 👎.
| if not include_all: | ||
| preview["wiki_rows"] = [] | ||
| return JSONResponse(preview) |
| if ( | ||
| DASHBOARD_PERMISSION_PROJECTS_READ not in permissions | ||
| and await asyncio.to_thread( | ||
| _session_can_view_any_project, | ||
| session, |
michaelmwu
force-pushed
the
michaelmwu/erp-project-status-view
branch
from
ff7a901 to
06d537a
Compare
There was a problem hiding this comment.
Actionable comments posted: 1
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@apps/api/src/five08/backend/api.py`:
- Around line 2675-2677: The preview currently clears preview["wiki_rows"] when
include_all is false but still leaks raw wiki table content via
preview["matches"][...]["best_match"]["row"]; after the existing
preview["wiki_rows"] = [] branch, iterate preview.get("matches", []) and for
each match where "best_match" exists remove or redact the "row" key (e.g., del
match["best_match"]["row"] or set it to None) so roster-limited users no longer
receive raw wiki rows; ensure you handle missing keys safely (use .get and
checks) to avoid KeyError before returning JSONResponse(preview).
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: defaults
Review profile: CHILL
Plan: Pro
Run ID: e1ed78dc-843e-4c17-9f72-1c223c303970
📒 Files selected for processing (24)
.env.exampleapps/admin_dashboard/src/main.tsxapps/api/src/five08/backend/api.pyapps/api/src/five08/backend/auth.pyapps/api/src/five08/backend/dashboard.pyapps/api/src/five08/backend/static/dashboard/.vite/manifest.jsonapps/api/src/five08/backend/static/dashboard/assets/index-CHUx-bdo.jsapps/api/src/five08/backend/static/dashboard/assets/index-Cdw3ivTK.cssapps/api/src/five08/backend/static/dashboard/assets/index-DfimwzZZ.cssapps/api/src/five08/backend/static/dashboard/assets/index-k4LODCF_.jsapps/api/src/five08/backend/static/dashboard/index.htmlapps/discord_bot/src/five08/discord_bot/cogs/admin_login.pyapps/discord_bot/src/five08/discord_bot/cogs/projects.pyapps/worker/src/five08/worker/erpnext_project_sync.pyapps/worker/src/five08/worker/jobs.pyapps/worker/src/five08/worker/migrations/versions/20260519_0100_create_project_cache_tables.pypackages/shared/src/five08/clients/__init__.pypackages/shared/src/five08/clients/erpnext.pypackages/shared/src/five08/projects.pypackages/shared/src/five08/settings.pytests/unit/test_admin_login_cog.pytests/unit/test_backend_api.pytests/unit/test_projects.pytests/unit/test_worker_erpnext_project_sync.py
💤 Files with no reviewable changes (1)
- apps/api/src/five08/backend/static/dashboard/assets/index-Cdw3ivTK.css
✅ Files skipped from review due to trivial changes (3)
- apps/api/src/five08/backend/static/dashboard/index.html
- apps/api/src/five08/backend/dashboard.py
- apps/api/src/five08/backend/static/dashboard/.vite/manifest.json
🚧 Files skipped from review as they are similar to previous changes (9)
- packages/shared/src/five08/settings.py
- apps/api/src/five08/backend/auth.py
- apps/worker/src/five08/worker/jobs.py
- packages/shared/src/five08/clients/init.py
- apps/discord_bot/src/five08/discord_bot/cogs/projects.py
- packages/shared/src/five08/clients/erpnext.py
- apps/worker/src/five08/worker/migrations/versions/20260519_0100_create_project_cache_tables.py
- tests/unit/test_backend_api.py
- apps/admin_dashboard/src/main.tsx
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 06d537a331
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
| window.history.pushState( | ||
| { view: "projects", projectId }, | ||
| "", | ||
| `/dashboard/projects/${encodeURIComponent(projectId)}`, |
There was a problem hiding this comment.
When a user opens a project detail and then refreshes or shares the URL, this pushes /dashboard/projects/{id}, but create_app() only registers the dashboard shell for /dashboard, /dashboard/{view}, and the existing /dashboard/gigs/{item_id} detail route; there is no matching /dashboard/projects/{item_id} route, so the browser gets a backend 404 instead of the React app. Add the same shell route for project detail paths before relying on these URLs.
Useful? React with 👍 / 👎.
michaelmwu
force-pushed
the
michaelmwu/erp-project-status-view
branch
from
06d537a to
e5e0c70
Compare
michaelmwu
force-pushed
the
michaelmwu/erp-project-status-view
branch
2 times, most recently
from
7532466 to
e7c2e0c
Compare
michaelmwu
force-pushed
the
michaelmwu/erp-project-status-view
branch
from
e7c2e0c to
72f478e
Compare
| function projectsUrl() { | ||
| const params = new URLSearchParams({ limit: "100" }) | ||
| if (projectQuery.trim()) params.set("query", projectQuery.trim()) | ||
| if (projectStatus) params.set("status", projectStatus) |
michaelmwu
force-pushed
the
michaelmwu/erp-project-status-view
branch
from
72f478e to
b64be43
Compare
michaelmwu
force-pushed
the
michaelmwu/erp-project-status-view
branch
from
b64be43 to
6c5be70
Compare
| function projectsUrl() { | ||
| const params = new URLSearchParams({ limit: "100", status: projectStatus }) | ||
| if (projectQuery.trim()) params.set("query", projectQuery.trim()) | ||
| return `/dashboard/api/projects?${params.toString()}` | ||
| } | ||
|
|
| client = _erpnext_client() | ||
| for project_id in project_ids: | ||
| project = _cached_dashboard_project_by_id(project_id) | ||
| if project is None: | ||
| failures.append( | ||
| {"project_id": project_id, "error": "project_not_found"} | ||
| ) | ||
| continue | ||
| external_project_id = str(project.get("erpnext_project_id") or "").strip() |
| client = ERPNextClient( | ||
| base_url, | ||
| api_key, | ||
| timeout_seconds=settings.erpnext_api_timeout_seconds, | ||
| ) | ||
| try: | ||
| suppliers = client.search_suppliers(normalized_email, limit=1) | ||
| except ERPNextAPIError: | ||
| return None | ||
| finally: | ||
| client.close() | ||
|
|
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 6c5be70cb0
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
| const [gigStatus, setGigStatus] = useState("") | ||
| const [gigLimit, setGigLimit] = useState(100) | ||
| const [projectQuery, setProjectQuery] = useState("") | ||
| const [projectStatus, setProjectStatus] = useState("Open") |
There was a problem hiding this comment.
When the dashboard loads /dashboard/projects/{id}, the client always initializes projectStatus to "Open", so the first fetch excludes completed/cancelled projects. As a result, refreshing or sharing a detail URL for a non-open project lands on the “not in the current result set” fallback even though the project exists, which breaks direct-link navigation for closed projects.
Useful? React with 👍 / 👎.
michaelmwu
force-pushed
the
michaelmwu/erp-project-status-view
branch
from
6c5be70 to
9849335
Compare
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 9849335d10
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
| if not normalized_ids: | ||
| return 0 |
There was a problem hiding this comment.
Remove the early return on empty seen_external_ids: when ERPNext legitimately has no open projects, sync_open_projects() still calls this helper, but if not normalized_ids: return 0 skips the update entirely and leaves every previously cached Open project stuck as open. This means a full close-out state in ERP will never propagate to the dashboard/Discord cache.
Useful? React with 👍 / 👎.
Summary
/projectsbacked by the synced project cache, with Steering seeing all projects and non-Steering users seeing only projects where their synced people-cache email matches the ERP roster.Access Model
Projects remain restricted to Steering/Admin by default. Regular users receive project visibility only when the cached ERP roster confirms membership for their email/source user id, and their dashboard/API/bot results are filtered to those matched projects. Non-Steering users do not receive global project summary counts or raw wiki rows.
Validation
./scripts/test.sh-> 1403 passed, 20 skipped./scripts/lint.sh./scripts/mypy.shbun run lintbun run buildNotes
ERP and wiki write-back are intentionally not included yet. The cache and external-id model are set up so a future mutable ERP key/status write flow can be added without treating current ERP project names as permanent identity.
Summary by CodeRabbit
New Features
/projectscommand for viewing accessible projectsChores