Hide Unpublished Programs and Modules From Public View #3117
Conversation
Summary by CodeRabbit
✏️ Tip: You can customize this high-level summary in your review settings. WalkthroughAdds backend checks to prevent public access to unpublished programs and modules, a GraphQL resolver cache invalidation utility and signal handler, and frontend updates to show Not Found/unavailable states and filter listings to published programs only. Changes
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~20 minutes Possibly related PRs
Suggested reviewers
🚥 Pre-merge checks | ✅ 5✅ Passed checks (5 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing touches
🧪 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 |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 2
🧹 Nitpick comments (2)
backend/apps/mentorship/api/internal/queries/module.py (1)
21-43: Access control properly implemented.The function correctly blocks access to modules from unpublished programs and logs security-relevant attempts. The empty-list return for missing/unpublished programs provides a consistent public API.
Optional: Minor efficiency improvement
Since you've already fetched the
programobject at line 27, you could useprogram.modulesinstead of filtering byprogram__keyagain:- return ( - Module.objects.filter(program__key=program_key) - .select_related("program", "project") - .prefetch_related("mentors__github_user") - .order_by("started_at") - ) + return ( + program.modules + .select_related("program", "project") + .prefetch_related("mentors__github_user") + .order_by("started_at") + )frontend/src/app/mentorship/programs/[programKey]/modules/[moduleKey]/page.tsx (1)
50-50: Consider whethermoduleKeyandprogramKeydependencies are necessary.Apollo Client automatically refetches the query when
variableschange (which includemoduleKeyandprogramKey). Including them in theuseEffectdependencies may be redundant and could cause the effect to run with staledatabefore the new query completes.During navigation between modules, the effect might briefly process old module data with new parameter values, though this doesn't leak unpublished content since the backend controls what's returned.
🔎 Simplified dependency array (if no specific edge case requires the extra deps)
- }, [data, error, moduleKey, programKey]) + }, [data, error])If these dependencies address a specific edge case, consider adding a comment explaining why they're needed.
📜 Review details
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (7)
backend/apps/core/utils/index.pybackend/apps/mentorship/api/internal/queries/module.pybackend/apps/mentorship/api/internal/queries/program.pybackend/apps/mentorship/signals/program.pyfrontend/src/app/mentorship/programs/[programKey]/modules/[moduleKey]/page.tsxfrontend/src/app/mentorship/programs/[programKey]/page.tsxfrontend/src/app/mentorship/programs/page.tsx
🧰 Additional context used
🧠 Learnings (8)
📚 Learning: 2025-08-31T13:47:15.861Z
Learnt from: rudransh-shrivastava
Repo: OWASP/Nest PR: 2155
File: frontend/src/server/queries/programsQueries.ts:81-81
Timestamp: 2025-08-31T13:47:15.861Z
Learning: In frontend/src/server/queries/programsQueries.ts, GET_PROGRAM_DETAILS is actively used in frontend/src/app/my/mentorship/programs/[programKey]/edit/page.tsx for program editing functionality and cannot be removed. It serves a different purpose than GET_PROGRAM_ADMIN_DETAILS, providing comprehensive program information needed for editing.
Applied to files:
frontend/src/app/mentorship/programs/page.tsxfrontend/src/app/mentorship/programs/[programKey]/modules/[moduleKey]/page.tsxfrontend/src/app/mentorship/programs/[programKey]/page.tsxbackend/apps/mentorship/api/internal/queries/module.pybackend/apps/mentorship/api/internal/queries/program.py
📚 Learning: 2025-08-31T13:47:15.861Z
Learnt from: rudransh-shrivastava
Repo: OWASP/Nest PR: 2155
File: frontend/src/server/queries/programsQueries.ts:81-81
Timestamp: 2025-08-31T13:47:15.861Z
Learning: In frontend/src/server/queries/programsQueries.ts, GET_PROGRAM_DETAILS and GET_PROGRAM_ADMIN_DETAILS are two separate queries serving different purposes: GET_PROGRAM_DETAILS fetches comprehensive program information while GET_PROGRAM_ADMIN_DETAILS fetches only admin-related details. These queries cannot be removed or merged as they serve different use cases in the application.
Applied to files:
frontend/src/app/mentorship/programs/page.tsxfrontend/src/app/mentorship/programs/[programKey]/page.tsxbackend/apps/mentorship/api/internal/queries/program.py
📚 Learning: 2025-08-31T13:47:15.861Z
Learnt from: rudransh-shrivastava
Repo: OWASP/Nest PR: 2155
File: frontend/src/server/queries/programsQueries.ts:81-81
Timestamp: 2025-08-31T13:47:15.861Z
Learning: In frontend/src/server/queries/programsQueries.ts, GET_PROGRAM_DETAILS and GET_PROGRAM_ADMIN_DETAILS are two separate queries serving different purposes: GET_PROGRAM_DETAILS fetches comprehensive program information while GET_PROGRAM_ADMIN_DETAILS fetches only admin-related details.
Applied to files:
frontend/src/app/mentorship/programs/page.tsxfrontend/src/app/mentorship/programs/[programKey]/page.tsxbackend/apps/mentorship/api/internal/queries/program.py
📚 Learning: 2025-09-29T06:02:35.566Z
Learnt from: rudransh-shrivastava
Repo: OWASP/Nest PR: 2178
File: frontend/src/components/SingleModuleCard.tsx:54-54
Timestamp: 2025-09-29T06:02:35.566Z
Learning: In the Module type from types/mentorship.ts, the experienceLevel field is required (experienceLevel: ExperienceLevelEnum), not optional, so null/undefined checks are not needed when accessing this field.
Applied to files:
frontend/src/app/mentorship/programs/[programKey]/modules/[moduleKey]/page.tsx
📚 Learning: 2025-07-12T17:14:28.536Z
Learnt from: Rajgupta36
Repo: OWASP/Nest PR: 1717
File: frontend/src/app/mentorship/programs/[programKey]/edit/page.tsx:90-128
Timestamp: 2025-07-12T17:14:28.536Z
Learning: Both ProgramForm (programCard.tsx) and ModuleForm (mainmoduleCard.tsx) components already implement HTML validation using the `required` attribute on form fields. The browser's native validation prevents form submission and displays error messages when required fields are empty, eliminating the need for additional JavaScript validation before GraphQL mutations.
Applied to files:
frontend/src/app/mentorship/programs/[programKey]/modules/[moduleKey]/page.tsxfrontend/src/app/mentorship/programs/[programKey]/page.tsx
📚 Learning: 2025-12-31T05:17:39.659Z
Learnt from: kart-u
Repo: OWASP/Nest PR: 3101
File: backend/apps/common/extensions.py:92-98
Timestamp: 2025-12-31T05:17:39.659Z
Learning: In this codebase, import OperationType for GraphQL operations from the graphql-core package rather than from strawberry. Use 'from graphql import OperationType'. Strawberry re-exports via graphql-core internally, so relying on strawberry's API may be brittle. Apply this rule to all Python files that deal with GraphQL operation types; ensure imports come from graphql (graphql-core) and not from strawberry packages. This improves compatibility and avoids coupling to strawberry's internals.
Applied to files:
backend/apps/core/utils/index.pybackend/apps/mentorship/signals/program.pybackend/apps/mentorship/api/internal/queries/module.pybackend/apps/mentorship/api/internal/queries/program.py
📚 Learning: 2025-07-11T15:57:56.648Z
Learnt from: Rajgupta36
Repo: OWASP/Nest PR: 1717
File: backend/apps/mentorship/graphql/queries/module.py:39-50
Timestamp: 2025-07-11T15:57:56.648Z
Learning: In the OWASP Nest mentorship GraphQL queries, Strawberry GraphQL automatically converts between Django Module instances and ModuleNode types, so methods can return Module instances directly without manual conversion even when typed as ModuleNode.
Applied to files:
backend/apps/mentorship/api/internal/queries/module.py
📚 Learning: 2025-07-13T05:55:46.436Z
Learnt from: Rajgupta36
Repo: OWASP/Nest PR: 1717
File: backend/apps/mentorship/graphql/mutations/program.py:166-166
Timestamp: 2025-07-13T05:55:46.436Z
Learning: In the OWASP Nest mentorship GraphQL mutations, Strawberry GraphQL automatically converts between Django Program instances and ProgramNode types, so mutations can return Program instances directly without manual conversion even when typed as ProgramNode, similar to the Module/ModuleNode pattern.
Applied to files:
backend/apps/mentorship/api/internal/queries/module.py
🧬 Code graph analysis (6)
frontend/src/app/mentorship/programs/page.tsx (1)
backend/apps/mentorship/api/internal/nodes/enum.py (1)
ProgramStatusEnum(22-27)
frontend/src/app/mentorship/programs/[programKey]/modules/[moduleKey]/page.tsx (1)
frontend/src/app/global-error.tsx (2)
handleAppError(67-91)ErrorDisplay(28-52)
backend/apps/mentorship/signals/program.py (2)
backend/apps/core/utils/index.py (2)
clear_index_cache(236-262)invalidate_program_graphql_cache(264-279)backend/apps/mentorship/models/program.py (1)
Program(18-87)
frontend/src/app/mentorship/programs/[programKey]/page.tsx (1)
frontend/src/app/global-error.tsx (1)
ErrorDisplay(28-52)
backend/apps/mentorship/api/internal/queries/module.py (2)
backend/apps/mentorship/models/program.py (2)
Program(18-87)ProgramStatus(25-28)backend/apps/mentorship/models/module.py (1)
Module(17-99)
backend/apps/mentorship/api/internal/queries/program.py (1)
backend/apps/mentorship/models/program.py (2)
Program(18-87)ProgramStatus(25-28)
🪛 Ruff (0.14.10)
backend/apps/core/utils/index.py
267-267: Missing blank line after last section ("Args")
Add blank line after "Args"
(D413)
276-276: Undefined name json
(F821)
277-277: Undefined name hashlib
(F821)
279-279: No newline at end of file
Add trailing newline
(W292)
backend/apps/mentorship/signals/program.py
32-32: First argument of a method should be named self
Rename sender to self
(N805)
🔇 Additional comments (10)
backend/apps/mentorship/api/internal/queries/module.py (3)
10-10: LGTM—Program import added correctly.The Program model import is necessary for the new status checks and properly used throughout the file.
61-90: LGTM—Comprehensive access control with proper error handling.The function correctly enforces published-only access and provides uniform error messages across all failure paths (unpublished, missing program, missing module). The warning logs will help track unauthorized access attempts.
46-58: PublishedModuleManager correctly filters by program publication status.The manager implementation at
backend/apps/mentorship/models/managers/module.pyconfirms thatget_queryset()filters byprogram__status=Program.ProgramStatus.PUBLISHED. The switch toModule.published_modulesinget_project_modules()is correct and ensures only modules from published programs are returned.backend/apps/mentorship/api/internal/queries/program.py (1)
23-45: LGTM—Access control correctly blocks unpublished programs.The status check properly restricts public access to published programs only. The consistent error messages and warning logs align well with the module access controls in the related file.
backend/apps/mentorship/signals/program.py (1)
31-42: LGTM—GraphQL cache invalidation integrated correctly.The signal handler properly clears GraphQL cache when a program is saved, ensuring cached queries reflect the latest program status. The logging provides good observability.
Note: The Ruff N805 warning about
senderis a false positive—Django signal handlers usesenderas the first parameter by convention, notself.frontend/src/app/mentorship/programs/[programKey]/modules/[moduleKey]/page.tsx (3)
20-49: LGTM! Error handling properly implements unpublished module restrictions.The addition of the
hasErrorstate and the extended handling logic correctly addresses the PR objectives:
- Lines 30-37: Properly handle GraphQL errors by calling
handleAppError, clearing state, and setting the error flag.- Lines 44-49: Crucially handle the case where the backend returns successfully but
getModuleis null (unpublished or non-existent modules), ensuring these are treated as "not found."This implementation ensures that unpublished modules are never displayed and users receive appropriate 404 responses.
41-41: Good addition of optional chaining for safety.The optional chaining (
data.getProgram?.admins || null) prevents potential crashes ifgetProgramis null or undefined, which could occur when a program is unpublished or unavailable.
54-59: Improved error condition and message clarity.The updated condition (
hasError || !module) makes the error state more explicit, and the revised message ("doesn't exist or is not available") appropriately communicates both missing and unpublished module scenarios to users.frontend/src/app/mentorship/programs/[programKey]/page.tsx (1)
22-22: Excellent error handling for unpublished programs.The changes correctly handle three cases:
- GraphQL errors (line 55): Caught and displayed as 404
- Null
getProgramresponse (line 68): Treated as an error, which is correct when the backend returns null for unpublished programs- Loading state management (lines 79-81): The
hasAttemptedLoadflag prevents premature rendering of the error state before the initial query completesThe error message "doesn't exist or is not available" (line 88) is well-chosen—it doesn't leak information about whether a program exists but is unpublished versus truly not existing.
The dependency array (line 77) correctly includes
errorandprogramKey, ensuring the component re-processes when the route changes or error state updates.Also applies to: 34-35, 55-77, 79-90
frontend/src/app/mentorship/programs/page.tsx (1)
36-39: Remove the redundant client-side filter for code clarity.The backend Algolia index for programs is explicitly pre-filtered to include only
PUBLISHEDstatus programs (Program.objects.filter(status=Program.ProgramStatus.PUBLISHED)inProgramIndex.get_entities()). This means the client-side filter at lines 36-39 is redundant—all items returned from the search are already published.Additionally, the frontend pagination and result counts are correct because they reflect only published programs from the index, not all programs in the database.
The client-side filter can be safely removed:
const publishedPrograms = programsNote:
COMPLETEDprograms are intentionally excluded from the index (backend design), not as a result of this client-side filter.Likely an incorrect or invalid review comment.
mrkeshav-05
force-pushed
the
fix/hide-public-view
branch
from
e624c5e to
c352495
Compare
There was a problem hiding this comment.
Actionable comments posted: 0
🧹 Nitpick comments (3)
backend/apps/mentorship/api/internal/queries/module.py (2)
26-43: Consider using the already-fetched program object in the query.After fetching the
programobject on line 27, the query on line 39 still filters byprogram__key=program_key, which causes an extra join. You can use the fetched object directly.🔎 Suggested optimization
except Program.DoesNotExist: return [] return ( - Module.objects.filter(program__key=program_key) + Module.objects.filter(program=program) .select_related("program", "project") .prefetch_related("mentors__github_user") .order_by("started_at") )
59-89: Good security-conscious error handling.The uniform error messages across all failure cases (unpublished, program not found, module not found) correctly prevent information leakage about publication status. The logging provides adequate context for debugging while keeping user-facing errors opaque.
Similar to
get_program_modules, the query on line 80 could useprogram_id=program.idinstead ofprogram__key=program_keyto avoid the redundant join.🔎 Suggested optimization
return ( Module.objects.select_related("program", "project") .prefetch_related("mentors__github_user") - .get(key=module_key, program__key=program_key) + .get(key=module_key, program=program) )backend/apps/core/utils/index.py (1)
268-286: Consider adding error handling and input validation for robustness.The function is called from a
post_savesignal handler inbackend/apps/mentorship/signals/program.py. While the cache key format is correct and matches the GraphQL resolver implementation, adding defensive measures would improve reliability:
- Add a try-except block around cache operations to prevent exceptions from propagating to the signal handler
- Validate that
program_keyis non-empty before proceedingSuggested improvements
def invalidate_program_graphql_cache(program_key: str) -> None: """Invalidate GraphQL cache for a specific program. Args: program_key: The program's unique key. """ + if not program_key: + logger.warning("Cannot invalidate GraphQL cache: program_key is empty") + return + queries_to_invalidate = [ ("getProgram", {"programKey": program_key}), ("getProgramModules", {"programKey": program_key}), ] - for field_name, field_args in queries_to_invalidate: - key = f"{field_name}:{json.dumps(field_args, sort_keys=True)}" - cache_key = ( - f"{settings.GRAPHQL_RESOLVER_CACHE_PREFIX}-{hashlib.sha256(key.encode()).hexdigest()}" - ) - cache.delete(cache_key) - logger.info("Invalidated GraphQL cache for %s with key: %s", field_name, program_key) + try: + for field_name, field_args in queries_to_invalidate: + key = f"{field_name}:{json.dumps(field_args, sort_keys=True)}" + cache_key = ( + f"{settings.GRAPHQL_RESOLVER_CACHE_PREFIX}-{hashlib.sha256(key.encode()).hexdigest()}" + ) + cache.delete(cache_key) + logger.info("Invalidated GraphQL cache for %s with key: %s", field_name, program_key) + except Exception as e: + logger.error("Failed to invalidate GraphQL cache for program %s: %s", program_key, e)
📜 Review details
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (3)
backend/apps/core/utils/index.pybackend/apps/mentorship/api/internal/queries/module.pybackend/apps/mentorship/signals/program.py
🚧 Files skipped from review as they are similar to previous changes (1)
- backend/apps/mentorship/signals/program.py
🧰 Additional context used
🧠 Learnings (5)
📓 Common learnings
Learnt from: Rajgupta36
Repo: OWASP/Nest PR: 1717
File: frontend/src/components/ModuleForm.tsx:112-134
Timestamp: 2025-07-14T16:18:07.287Z
Learning: In the OWASP Nest mentorship system, date validation for modules (ensuring start date precedes end date) is handled on the backend in the module GraphQL mutations via the `_validate_module_dates` helper function in backend/apps/mentorship/graphql/mutations/module.py, which prevents invalid date ranges from being stored in the database.
📚 Learning: 2025-12-31T05:17:39.659Z
Learnt from: kart-u
Repo: OWASP/Nest PR: 3101
File: backend/apps/common/extensions.py:92-98
Timestamp: 2025-12-31T05:17:39.659Z
Learning: In this codebase, import OperationType for GraphQL operations from the graphql-core package rather than from strawberry. Use 'from graphql import OperationType'. Strawberry re-exports via graphql-core internally, so relying on strawberry's API may be brittle. Apply this rule to all Python files that deal with GraphQL operation types; ensure imports come from graphql (graphql-core) and not from strawberry packages. This improves compatibility and avoids coupling to strawberry's internals.
Applied to files:
backend/apps/core/utils/index.pybackend/apps/mentorship/api/internal/queries/module.py
📚 Learning: 2025-08-31T13:47:15.861Z
Learnt from: rudransh-shrivastava
Repo: OWASP/Nest PR: 2155
File: frontend/src/server/queries/programsQueries.ts:81-81
Timestamp: 2025-08-31T13:47:15.861Z
Learning: In frontend/src/server/queries/programsQueries.ts, GET_PROGRAM_DETAILS is actively used in frontend/src/app/my/mentorship/programs/[programKey]/edit/page.tsx for program editing functionality and cannot be removed. It serves a different purpose than GET_PROGRAM_ADMIN_DETAILS, providing comprehensive program information needed for editing.
Applied to files:
backend/apps/mentorship/api/internal/queries/module.py
📚 Learning: 2025-07-11T15:57:56.648Z
Learnt from: Rajgupta36
Repo: OWASP/Nest PR: 1717
File: backend/apps/mentorship/graphql/queries/module.py:39-50
Timestamp: 2025-07-11T15:57:56.648Z
Learning: In the OWASP Nest mentorship GraphQL queries, Strawberry GraphQL automatically converts between Django Module instances and ModuleNode types, so methods can return Module instances directly without manual conversion even when typed as ModuleNode.
Applied to files:
backend/apps/mentorship/api/internal/queries/module.py
📚 Learning: 2025-07-13T05:55:46.436Z
Learnt from: Rajgupta36
Repo: OWASP/Nest PR: 1717
File: backend/apps/mentorship/graphql/mutations/program.py:166-166
Timestamp: 2025-07-13T05:55:46.436Z
Learning: In the OWASP Nest mentorship GraphQL mutations, Strawberry GraphQL automatically converts between Django Program instances and ProgramNode types, so mutations can return Program instances directly without manual conversion even when typed as ProgramNode, similar to the Module/ModuleNode pattern.
Applied to files:
backend/apps/mentorship/api/internal/queries/module.py
🧬 Code graph analysis (2)
backend/apps/core/utils/index.py (2)
backend/apps/common/index.py (1)
register(104-123)backend/apps/owasp/api/internal/nodes/project.py (1)
key(68-70)
backend/apps/mentorship/api/internal/queries/module.py (3)
backend/apps/mentorship/models/program.py (2)
Program(18-87)ProgramStatus(25-28)backend/apps/mentorship/models/module.py (1)
Module(17-99)backend/apps/mentorship/api/internal/nodes/module.py (1)
ModuleNode(19-159)
🔇 Additional comments (2)
backend/apps/mentorship/api/internal/queries/module.py (1)
45-57: LGTM! Clean use of the published_modules manager.The implementation correctly leverages the
PublishedModuleManagerto filter modules from published programs (viaprogram__status=PUBLISHED), keeping the query code clean and maintainable.backend/apps/core/utils/index.py (1)
4-5: Imports correctly added.The missing imports (
hashlib,json, andsettings) flagged in the previous review have been properly added. This resolves the runtimeNameErrorissue.Also applies to: 11-11
mrkeshav-05
force-pushed
the
fix/hide-public-view
branch
from
90ed77e to
9e403f5
Compare
mrkeshav-05
force-pushed
the
fix/hide-public-view
branch
from
fc2ff37 to
4528b64
Compare
There was a problem hiding this comment.
Actionable comments posted: 0
🧹 Nitpick comments (1)
frontend/src/app/mentorship/programs/[programKey]/page.tsx (1)
82-93: Refactor dependencies to avoid fragile state cycle.Including
hasError,hasAttemptedLoad, andprogramin the dependencies array creates a pattern where the effect can re-trigger itself after updating these state variables. While the current guards (lines 57, 67, 73) prevent infinite loops, this is fragile:
- Future modifications to the guards could introduce infinite re-renders
- ESLint exhaustive-deps rules may flag this pattern
- The intent is harder to understand for maintainers
🔎 Suggested approaches to simplify
Option 1: Use refs for values that don't need to trigger effects
+ const hasErrorRef = useRef(false) + const hasAttemptedLoadRef = useRef(false) useEffect(() => { const processResult = async () => { // ... if (error) { - if (!hasError) { + if (!hasErrorRef.current) { - setHasError(true) + hasErrorRef.current = true + setHasError(true) setProgram(null) setModules([]) - setHasAttemptedLoad(true) + hasAttemptedLoadRef.current = true + setHasAttemptedLoad(true) } return } // similar changes for other branches... } processResult() - }, [shouldRefresh, data, error, refetch, router, searchParams, programKey, hasError, hasAttemptedLoad, program]) + }, [shouldRefresh, data, error, refetch, router, searchParams, programKey])Option 2: Use a reducer to manage related state
Consider consolidating
hasError,hasAttemptedLoad,program, andmodulesinto a single reducer state to eliminate interdependencies.
📜 Review details
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (1)
frontend/src/app/mentorship/programs/[programKey]/page.tsx
🧰 Additional context used
🧠 Learnings (5)
📓 Common learnings
Learnt from: rudransh-shrivastava
Repo: OWASP/Nest PR: 2155
File: frontend/src/server/queries/programsQueries.ts:81-81
Timestamp: 2025-08-31T13:47:15.861Z
Learning: In frontend/src/server/queries/programsQueries.ts, GET_PROGRAM_DETAILS is actively used in frontend/src/app/my/mentorship/programs/[programKey]/edit/page.tsx for program editing functionality and cannot be removed. It serves a different purpose than GET_PROGRAM_ADMIN_DETAILS, providing comprehensive program information needed for editing.
📚 Learning: 2025-08-31T13:47:15.861Z
Learnt from: rudransh-shrivastava
Repo: OWASP/Nest PR: 2155
File: frontend/src/server/queries/programsQueries.ts:81-81
Timestamp: 2025-08-31T13:47:15.861Z
Learning: In frontend/src/server/queries/programsQueries.ts, GET_PROGRAM_DETAILS is actively used in frontend/src/app/my/mentorship/programs/[programKey]/edit/page.tsx for program editing functionality and cannot be removed. It serves a different purpose than GET_PROGRAM_ADMIN_DETAILS, providing comprehensive program information needed for editing.
Applied to files:
frontend/src/app/mentorship/programs/[programKey]/page.tsx
📚 Learning: 2025-08-31T13:47:15.861Z
Learnt from: rudransh-shrivastava
Repo: OWASP/Nest PR: 2155
File: frontend/src/server/queries/programsQueries.ts:81-81
Timestamp: 2025-08-31T13:47:15.861Z
Learning: In frontend/src/server/queries/programsQueries.ts, GET_PROGRAM_DETAILS and GET_PROGRAM_ADMIN_DETAILS are two separate queries serving different purposes: GET_PROGRAM_DETAILS fetches comprehensive program information while GET_PROGRAM_ADMIN_DETAILS fetches only admin-related details. These queries cannot be removed or merged as they serve different use cases in the application.
Applied to files:
frontend/src/app/mentorship/programs/[programKey]/page.tsx
📚 Learning: 2025-08-31T13:47:15.861Z
Learnt from: rudransh-shrivastava
Repo: OWASP/Nest PR: 2155
File: frontend/src/server/queries/programsQueries.ts:81-81
Timestamp: 2025-08-31T13:47:15.861Z
Learning: In frontend/src/server/queries/programsQueries.ts, GET_PROGRAM_DETAILS and GET_PROGRAM_ADMIN_DETAILS are two separate queries serving different purposes: GET_PROGRAM_DETAILS fetches comprehensive program information while GET_PROGRAM_ADMIN_DETAILS fetches only admin-related details.
Applied to files:
frontend/src/app/mentorship/programs/[programKey]/page.tsx
📚 Learning: 2025-07-12T17:14:28.536Z
Learnt from: Rajgupta36
Repo: OWASP/Nest PR: 1717
File: frontend/src/app/mentorship/programs/[programKey]/edit/page.tsx:90-128
Timestamp: 2025-07-12T17:14:28.536Z
Learning: Both ProgramForm (programCard.tsx) and ModuleForm (mainmoduleCard.tsx) components already implement HTML validation using the `required` attribute on form fields. The browser's native validation prevents form submission and displays error messages when required fields are empty, eliminating the need for additional JavaScript validation before GraphQL mutations.
Applied to files:
frontend/src/app/mentorship/programs/[programKey]/page.tsx
🧬 Code graph analysis (1)
frontend/src/app/mentorship/programs/[programKey]/page.tsx (1)
frontend/src/app/global-error.tsx (1)
ErrorDisplay(28-52)
🔇 Additional comments (7)
frontend/src/app/mentorship/programs/[programKey]/page.tsx (7)
20-22: LGTM!The
errorfield is correctly extracted from the query result to enable error handling for unpublished or unavailable programs.
34-35: State flags enable proper loading/error UX.The
hasErrorandhasAttemptedLoadflags correctly track whether data loading has been attempted and whether an error occurred, ensuring the UI doesn't prematurely show an error state before the first load attempt completes.
53-54: LGTM!The explicit early return after handling the refresh prevents further processing during the refresh cycle, which is the correct control flow.
56-64: LGTM!The error handling correctly processes GraphQL errors by clearing program data and setting error flags. The guard
if (!hasError)prevents redundant state updates when the error state is already established.
66-78: LGTM! Backend null response correctly handled.The logic correctly implements the PR objective:
- Lines 66-72: Loads program data only when necessary (initial load or programKey change)
- Lines 73-78: Treats backend returning
nullforgetProgramas a "not found" error, which correctly handles unpublished programs being inaccessible
95-97: LGTM!The loading condition correctly ensures the spinner displays until at least one data load attempt has completed, preventing a premature error state from being shown.
99-107: LGTM! Error messaging aligns with PR objectives.The error display condition and message correctly handle both error states and missing program data, effectively treating unpublished programs as "not available" with a 404 response, which aligns with the PR objective of hiding unpublished programs from public view.
kasya
left a comment
kasya
left a comment
There was a problem hiding this comment.
hi @mrkeshav-05 ! Left a request ⬇️
| if program.status != Program.ProgramStatus.PUBLISHED: | ||
| msg = f"Program with key '{program_key}' not found." | ||
| logger.warning( | ||
| "Attempted public access to unpublished program '%s' (status: %s)", | ||
| program_key, | ||
| program.status, | ||
| ) | ||
| raise ObjectDoesNotExist(msg) | ||
|
|
There was a problem hiding this comment.
@mrkeshav-05 programs and modules should still be available from admin view (/my/mentorship/) even if they are not published. Otherwise, how would mentors edit them? 🤔
Right now I'm not able to access any unpublished program in my admin view:
We only need to hide these from public view - /mentorship path. Same for modules.
There was a problem hiding this comment.
Also please don't forget to run make check-test before pushing changes 👌🏼
mrkeshav-05
force-pushed
the
fix/hide-public-view
branch
from
d1294e0 to
337d8bf
Compare
|
There was a problem hiding this comment.
Actionable comments posted: 1
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
backend/apps/mentorship/api/internal/queries/module.py (1)
59-89: Create an authenticated module query for admin access to unpublished programs.The admin module edit page (
/my/mentorship/programs/[programKey]/modules/[moduleKey]/edit/page.tsx) usesGetProgramAdminsAndModulesDocument, which calls the publicgetModulequery. This query requires the parent program to bePUBLISHED, blocking admins from editing modules in draft or completed programs.Unlike the program queries which have separate
GET_PROGRAM_DETAILSandGET_PROGRAM_ADMIN_DETAILSendpoints, the module API has only one public query. Create a separate authenticated query (e.g.,getModuleAdmin) that allows admins to fetch modules regardless of program status, or add an authenticated variant ofget_moduleto the query field.
🤖 Fix all issues with AI agents
In @backend/apps/mentorship/api/internal/queries/program.py:
- Around line 31-38: The public get_program resolver currently blocks
unpublished programs by checking Program.ProgramStatus.PUBLISHED, which breaks
admin edit flows; add an authenticated resolver my_program(programKey) that
fetches the Program by key without the published-status guard (reusing the same
lookup logic as update_program and my_programs) and enforce admin/owner
permission checks there, or alternatively modify get_program to only enforce the
published check when the request is unauthenticated (e.g., check
request.user.is_authenticated before raising ObjectDoesNotExist for unpublished
programs); update the frontend to call my_program for edit screens.
🧹 Nitpick comments (4)
backend/apps/mentorship/api/internal/queries/module.py (2)
26-43: Consider using the fetched program object for efficiency.The program is fetched on line 27 to check its status, but then the module query on line 39 performs another lookup via
program__key=program_key. You can reuse the fetched program object to avoid the redundant join.♻️ Suggested improvement
try: program = Program.objects.get(key=program_key) if program.status != Program.ProgramStatus.PUBLISHED: logger.warning( "Attempted to access modules for unpublished program '%s' (status: %s)", program_key, program.status, ) return [] except Program.DoesNotExist: return [] return ( - Module.objects.filter(program__key=program_key) + Module.objects.filter(program=program) .select_related("program", "project") .prefetch_related("mentors__github_user") .order_by("started_at") )
66-81: Same efficiency improvement applies here.Similar to
get_program_modules, the fetched program object can be reused in the module query.♻️ Suggested improvement
try: program = Program.objects.get(key=program_key) if program.status != Program.ProgramStatus.PUBLISHED: msg = f"Module with key '{module_key}' under program '{program_key}' not found." logger.warning( "Attempted to access module '%s' from unpublished program '%s' (status: %s)", module_key, program_key, program.status, ) raise ObjectDoesNotExist(msg) return ( Module.objects.select_related("program", "project") .prefetch_related("mentors__github_user") - .get(key=module_key, program__key=program_key) + .get(key=module_key, program=program) )frontend/src/app/mentorship/programs/[programKey]/modules/[moduleKey]/page.tsx (2)
50-50: Simplify dependency array to prevent stale data display.Including
moduleKeyandprogramKeyin the dependency array can cause the effect to briefly display stale data when navigating between modules:
- When params change, the effect runs with stale
data(old module)- The effect sets the old module temporarily
- Apollo refetches automatically and new data arrives
- The effect runs again and sets the correct module
Since Apollo Client automatically refetches when query variables change, and that refetch updates
dataanderror(which are already in the dependency array), including the params is redundant and causes an unnecessary intermediate state update.♻️ Recommended refactor
- }, [data, error, moduleKey, programKey]) + }, [data, error])This way, the effect only runs when the query result actually changes, preventing the brief flash of stale data during navigation.
19-19: Consider using Apollo's loading state for better navigation UX.The component manages its own
isLoadingstate but doesn't reset it when navigating between modules. This means the loading spinner won't show during navigation, potentially leaving users looking at stale content while new data loads.Consider using Apollo's built-in
loadingstate fromuseQuery:♻️ Suggested refactor
- const { data, error } = useQuery(GetProgramAdminsAndModulesDocument, { + const { data, error, loading } = useQuery(GetProgramAdminsAndModulesDocument, { variables: { programKey, moduleKey, }, }) // ... in useEffect, remove manual isLoading management - if (isLoading) return <LoadingSpinner /> + if (loading) return <LoadingSpinner />This automatically handles loading state during initial load and navigation between modules.
Also applies to: 52-52
📜 Review details
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (7)
backend/apps/core/utils/index.pybackend/apps/mentorship/api/internal/queries/module.pybackend/apps/mentorship/api/internal/queries/program.pybackend/apps/mentorship/signals/program.pyfrontend/src/app/mentorship/programs/[programKey]/modules/[moduleKey]/page.tsxfrontend/src/app/mentorship/programs/[programKey]/page.tsxfrontend/src/app/mentorship/programs/page.tsx
🚧 Files skipped from review as they are similar to previous changes (4)
- frontend/src/app/mentorship/programs/page.tsx
- backend/apps/mentorship/signals/program.py
- backend/apps/core/utils/index.py
- frontend/src/app/mentorship/programs/[programKey]/page.tsx
🧰 Additional context used
🧠 Learnings (10)
📓 Common learnings
Learnt from: Rajgupta36
Repo: OWASP/Nest PR: 1717
File: backend/apps/mentorship/graphql/mutations/program.py:41-44
Timestamp: 2025-07-08T17:24:36.501Z
Learning: In the mentorship program GraphQL mutations, date validation is handled at the GraphQL schema/node level in the input types (CreateProgramInput, UpdateProgramInput), preventing null values from reaching the mutation logic where date comparisons are performed.
Learnt from: Rajgupta36
Repo: OWASP/Nest PR: 1717
File: frontend/src/components/ModuleForm.tsx:112-134
Timestamp: 2025-07-14T16:18:07.287Z
Learning: In the OWASP Nest mentorship system, date validation for modules (ensuring start date precedes end date) is handled on the backend in the module GraphQL mutations via the `_validate_module_dates` helper function in backend/apps/mentorship/graphql/mutations/module.py, which prevents invalid date ranges from being stored in the database.
📚 Learning: 2025-08-31T13:47:15.861Z
Learnt from: rudransh-shrivastava
Repo: OWASP/Nest PR: 2155
File: frontend/src/server/queries/programsQueries.ts:81-81
Timestamp: 2025-08-31T13:47:15.861Z
Learning: In frontend/src/server/queries/programsQueries.ts, GET_PROGRAM_DETAILS is actively used in frontend/src/app/my/mentorship/programs/[programKey]/edit/page.tsx for program editing functionality and cannot be removed. It serves a different purpose than GET_PROGRAM_ADMIN_DETAILS, providing comprehensive program information needed for editing.
Applied to files:
frontend/src/app/mentorship/programs/[programKey]/modules/[moduleKey]/page.tsxbackend/apps/mentorship/api/internal/queries/module.pybackend/apps/mentorship/api/internal/queries/program.py
📚 Learning: 2025-09-29T06:02:35.566Z
Learnt from: rudransh-shrivastava
Repo: OWASP/Nest PR: 2178
File: frontend/src/components/SingleModuleCard.tsx:54-54
Timestamp: 2025-09-29T06:02:35.566Z
Learning: In the Module type from types/mentorship.ts, the experienceLevel field is required (experienceLevel: ExperienceLevelEnum), not optional, so null/undefined checks are not needed when accessing this field.
Applied to files:
frontend/src/app/mentorship/programs/[programKey]/modules/[moduleKey]/page.tsx
📚 Learning: 2025-07-12T17:14:28.536Z
Learnt from: Rajgupta36
Repo: OWASP/Nest PR: 1717
File: frontend/src/app/mentorship/programs/[programKey]/edit/page.tsx:90-128
Timestamp: 2025-07-12T17:14:28.536Z
Learning: Both ProgramForm (programCard.tsx) and ModuleForm (mainmoduleCard.tsx) components already implement HTML validation using the `required` attribute on form fields. The browser's native validation prevents form submission and displays error messages when required fields are empty, eliminating the need for additional JavaScript validation before GraphQL mutations.
Applied to files:
frontend/src/app/mentorship/programs/[programKey]/modules/[moduleKey]/page.tsx
📚 Learning: 2025-08-31T13:47:15.861Z
Learnt from: rudransh-shrivastava
Repo: OWASP/Nest PR: 2155
File: frontend/src/server/queries/programsQueries.ts:81-81
Timestamp: 2025-08-31T13:47:15.861Z
Learning: In frontend/src/server/queries/programsQueries.ts, GET_PROGRAM_DETAILS and GET_PROGRAM_ADMIN_DETAILS are two separate queries serving different purposes: GET_PROGRAM_DETAILS fetches comprehensive program information while GET_PROGRAM_ADMIN_DETAILS fetches only admin-related details. These queries cannot be removed or merged as they serve different use cases in the application.
Applied to files:
frontend/src/app/mentorship/programs/[programKey]/modules/[moduleKey]/page.tsxbackend/apps/mentorship/api/internal/queries/program.py
📚 Learning: 2025-08-31T13:47:15.861Z
Learnt from: rudransh-shrivastava
Repo: OWASP/Nest PR: 2155
File: frontend/src/server/queries/programsQueries.ts:81-81
Timestamp: 2025-08-31T13:47:15.861Z
Learning: In frontend/src/server/queries/programsQueries.ts, GET_PROGRAM_DETAILS and GET_PROGRAM_ADMIN_DETAILS are two separate queries serving different purposes: GET_PROGRAM_DETAILS fetches comprehensive program information while GET_PROGRAM_ADMIN_DETAILS fetches only admin-related details.
Applied to files:
frontend/src/app/mentorship/programs/[programKey]/modules/[moduleKey]/page.tsxbackend/apps/mentorship/api/internal/queries/program.py
📚 Learning: 2025-07-11T15:57:56.648Z
Learnt from: Rajgupta36
Repo: OWASP/Nest PR: 1717
File: backend/apps/mentorship/graphql/queries/module.py:39-50
Timestamp: 2025-07-11T15:57:56.648Z
Learning: In the OWASP Nest mentorship GraphQL queries, Strawberry GraphQL automatically converts between Django Module instances and ModuleNode types, so methods can return Module instances directly without manual conversion even when typed as ModuleNode.
Applied to files:
backend/apps/mentorship/api/internal/queries/module.py
📚 Learning: 2025-07-13T05:55:46.436Z
Learnt from: Rajgupta36
Repo: OWASP/Nest PR: 1717
File: backend/apps/mentorship/graphql/mutations/program.py:166-166
Timestamp: 2025-07-13T05:55:46.436Z
Learning: In the OWASP Nest mentorship GraphQL mutations, Strawberry GraphQL automatically converts between Django Program instances and ProgramNode types, so mutations can return Program instances directly without manual conversion even when typed as ProgramNode, similar to the Module/ModuleNode pattern.
Applied to files:
backend/apps/mentorship/api/internal/queries/module.py
📚 Learning: 2025-12-31T05:17:39.659Z
Learnt from: kart-u
Repo: OWASP/Nest PR: 3101
File: backend/apps/common/extensions.py:92-98
Timestamp: 2025-12-31T05:17:39.659Z
Learning: In this codebase, import OperationType for GraphQL operations from the graphql-core package rather than from strawberry. Use 'from graphql import OperationType'. Strawberry re-exports via graphql-core internally, so relying on strawberry's API may be brittle. Apply this rule to all Python files that deal with GraphQL operation types; ensure imports come from graphql (graphql-core) and not from strawberry packages. This improves compatibility and avoids coupling to strawberry's internals.
Applied to files:
backend/apps/mentorship/api/internal/queries/module.pybackend/apps/mentorship/api/internal/queries/program.py
📚 Learning: 2026-01-01T17:48:23.963Z
Learnt from: rudransh-shrivastava
Repo: OWASP/Nest PR: 2948
File: backend/apps/owasp/management/commands/owasp_generate_community_snapshot_video.py:41-47
Timestamp: 2026-01-01T17:48:23.963Z
Learning: In Django code, be aware that a QuerySet's boolean evaluation (e.g., if not queryset) runs a database query to determine emptiness. While it is technically valid to use the queryset in a boolean context, use queryset.exists() for existence checks to avoid unnecessary queries and improve performance. Applicable broadly to Python/Django files rather than just this specific path.
Applied to files:
backend/apps/mentorship/api/internal/queries/module.pybackend/apps/mentorship/api/internal/queries/program.py
🧬 Code graph analysis (3)
frontend/src/app/mentorship/programs/[programKey]/modules/[moduleKey]/page.tsx (1)
frontend/src/app/global-error.tsx (2)
handleAppError(67-91)ErrorDisplay(28-52)
backend/apps/mentorship/api/internal/queries/module.py (2)
backend/apps/mentorship/models/program.py (2)
Program(18-87)ProgramStatus(25-28)backend/apps/mentorship/models/module.py (1)
Module(17-99)
backend/apps/mentorship/api/internal/queries/program.py (1)
backend/apps/mentorship/models/program.py (2)
Program(18-87)ProgramStatus(25-28)
🔇 Additional comments (5)
backend/apps/mentorship/api/internal/queries/program.py (1)
40-45: LGTM!Exception handling is consistent with the unpublished program case, using uniform error messages and appropriate logging.
backend/apps/mentorship/api/internal/queries/module.py (1)
52-57: LGTM!Good use of the
Module.published_modulesmanager to encapsulate the published-program filtering logic, keeping this query clean and consistent.frontend/src/app/mentorship/programs/[programKey]/modules/[moduleKey]/page.tsx (3)
20-20: Well-structured error handling.The explicit
hasErrorstate and error handling logic are clear and correctly handle the error case by clearing state, stopping loading, and displaying appropriate feedback to users. This aligns well with the PR objective of hiding unpublished content.Note: Both
handleAppError(which shows a toast) andErrorDisplay(full-page error) will be shown on errors. This double notification might be intentional for user awareness, but consider if the toast is necessary when showing a full error page.Also applies to: 30-37
39-49: Correct implementation of unpublished module handling.The logic correctly handles both success and not-found cases:
- When
data.getModuleis null (unpublished/non-existent module), it properly sets error state- Explicitly clears error state on successful data retrieval
- The null fallback on line 41 ensures consistent state types
This implementation correctly supports the PR objective of hiding unpublished modules from public view.
54-62: Clear error display with appropriate messaging.The updated condition checks both
hasErrorand!module, making the error handling intent explicit. The error message appropriately mentions "doesn't exist or is not available," which aligns with the PR objective of hiding unpublished modules without revealing their existence.
| if program.status != Program.ProgramStatus.PUBLISHED: | ||
| msg = f"Program with key '{program_key}' not found." | ||
| logger.warning( | ||
| "Attempted public access to unpublished program '%s' (status: %s)", | ||
| program_key, | ||
| program.status, | ||
| ) | ||
| raise ObjectDoesNotExist(msg) |
There was a problem hiding this comment.
🧩 Analysis chain
🏁 Script executed:
# Check backend endpoints in program.py
cat -n backend/apps/mentorship/api/internal/queries/program.py | head -100Repository: OWASP/Nest
Length of output: 164
🏁 Script executed:
# Look for authenticated endpoints for single program access
rg -n "def (get_|my_)" backend/apps/mentorship/api/internal/queries/program.py -A 3Repository: OWASP/Nest
Length of output: 325
🏁 Script executed:
# Check frontend usage of GET_PROGRAM_DETAILS in edit pages
rg -n "GET_PROGRAM_DETAILS\|getProgram" frontend/src/app/my/mentorship/ -B 2 -A 5Repository: OWASP/Nest
Length of output: 36
🏁 Script executed:
# Check all GraphQL queries in programsQueries.ts to see all available queries
cat -n frontend/src/server/queries/programsQueries.tsRepository: OWASP/Nest
Length of output: 2586
🏁 Script executed:
# Get full implementation of get_program
sed -n '23,46p' backend/apps/mentorship/api/internal/queries/program.pyRepository: OWASP/Nest
Length of output: 1000
🏁 Script executed:
# Look for GraphQL resolvers for getProgram
find backend -name "*.py" -type f | xargs rg -l "getProgram|get_program" | head -20Repository: OWASP/Nest
Length of output: 197
🏁 Script executed:
# Check if there's a resolver file that maps GraphQL to Python
find backend -name "*resolver*" -o -name "*schema*" | grep -E "(resolver|schema)" | head -20Repository: OWASP/Nest
Length of output: 36
🏁 Script executed:
# Look for the schema definition and field resolvers
rg -n "getProgram|class Program" backend/apps/mentorship/ --type py | head -30Repository: OWASP/Nest
Length of output: 920
🏁 Script executed:
# Check the ProgramQuery class to see what queries are exposed
cat -n backend/apps/mentorship/api/internal/queries/program.pyRepository: OWASP/Nest
Length of output: 3893
🏁 Script executed:
# Check if there's a mutation to fetch a program for editing
cat -n backend/apps/mentorship/api/internal/mutations/program.pyRepository: OWASP/Nest
Length of output: 7045
🏁 Script executed:
# Look for authentication/authorization decorators or context checks
rg -n "authentication|permission|user_info|is_authenticated" backend/apps/mentorship/api/ --type py | head -20Repository: OWASP/Nest
Length of output: 1657
🏁 Script executed:
# Check the ProgramNode to understand the resolver setup
cat -n backend/apps/mentorship/api/internal/nodes/program.pyRepository: OWASP/Nest
Length of output: 2485
Add authenticated query to fetch single programs for editing.
The concern is valid—the new published status check in get_program() will break the admin edit workflow. The frontend's GET_PROGRAM_DETAILS query uses the public getProgram endpoint to fetch programs for editing, but this endpoint now blocks unpublished programs entirely. Admins cannot fetch their own DRAFT programs for editing.
The backend already demonstrates the correct pattern in update_program mutation: it fetches programs without a published status check and instead validates admin permission. Add an authenticated query (e.g., my_program(programKey)) that returns a single program by key without the published restriction, similar to my_programs, and update the frontend to use this for edit workflows. Alternatively, modify get_program to check authentication context and conditionally enforce the published requirement only for unauthenticated requests.
🤖 Prompt for AI Agents
In @backend/apps/mentorship/api/internal/queries/program.py around lines 31 -
38, The public get_program resolver currently blocks unpublished programs by
checking Program.ProgramStatus.PUBLISHED, which breaks admin edit flows; add an
authenticated resolver my_program(programKey) that fetches the Program by key
without the published-status guard (reusing the same lookup logic as
update_program and my_programs) and enforce admin/owner permission checks there,
or alternatively modify get_program to only enforce the published check when the
request is unauthenticated (e.g., check request.user.is_authenticated before
raising ObjectDoesNotExist for unpublished programs); update the frontend to
call my_program for edit screens.
2 participants
Proposed change
This PR fixes the issue where unpublished programs were accessible via direct URLs. Now, unpublished programs return a 404 "Page Not Found" error. Similarly, modules belonging to unpublished programs also return 404, ensuring draft content remains hidden from public view.
Resolves: #2859
Checklist
make check-testlocally and all tests passed