Hide Unpublished Programs and Modules From Public View#3117
Hide Unpublished Programs and Modules From Public View#3117kasya merged 13 commits intoOWASP:mainfrom
Conversation
|
Note Reviews pausedIt looks like this branch is under active development. To avoid overwhelming you with review comments due to an influx of new commits, CodeRabbit has automatically paused this review. You can configure this behavior by changing the Use the following commands to manage reviews:
Use the checkboxes below for quick actions:
WalkthroughThe pull request implements access control to restrict unpublished programs and modules from public view. Query methods now accept request context via Changes
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~25 minutes 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.
mrkeshav-05
force-pushed
the
fix/hide-public-view
branch
from
337d8bf to
a4c1999
Compare
There was a problem hiding this comment.
Actionable comments posted: 0
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)
20-43: Re-assert published status on the modules queryset (defense-in-depth + avoids TOCTOU).Even after the initial
Programstatus check, it’s safer to also constrain the module query itself (and removes any chance of returning modules if status flips between the two queries).Suggested tweak
return ( - Module.objects.filter(program__key=program_key) + Module.objects.filter( + program__key=program_key, + program__status=Program.ProgramStatus.PUBLISHED, + ) .select_related("program", "project") .prefetch_related("mentors__github_user") .order_by("started_at") )
🧹 Nitpick comments (2)
backend/apps/mentorship/api/internal/queries/program.py (1)
24-45: Consider pushing the “published-only” constraint into the queryset to avoid extra work + tighten TOCTOU.This avoids prefetching admins for an unpublished program and makes the status constraint part of the retrieval (single source of truth). Keep the warning log if you still want the “unpublished access attempt” signal (might require a separate cheap status lookup).
backend/apps/mentorship/api/internal/queries/module.py (1)
60-89:get_modulegating is correct; consider collapsing to a single DB query (optional) and keep the generic “not found” message.Current behavior matches the objective (no data disclosure; unpublished => “not found”). If you want fewer queries + tighter enforcement, you can fetch the module via a join constraint on
program__status(at the cost of losing the “unpublished access” log specificity unless you add a separate status lookup).Possible single-query approach
- 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__key=program_key, + program__status=Program.ProgramStatus.PUBLISHED, + ) ) - except Program.DoesNotExist as err: - msg = f"Module with key '{module_key}' under program '{program_key}' not found." - logger.warning(msg, exc_info=True) - raise ObjectDoesNotExist(msg) from err except Module.DoesNotExist as err: msg = f"Module with key '{module_key}' under program '{program_key}' not found." logger.warning(msg, exc_info=True) raise ObjectDoesNotExist(msg) from err
📜 Review details
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (5)
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/page.tsx
🚧 Files skipped from review as they are similar to previous changes (3)
- backend/apps/core/utils/index.py
- backend/apps/mentorship/signals/program.py
- frontend/src/app/mentorship/programs/page.tsx
🧰 Additional context used
🧠 Learnings (9)
📓 Common learnings
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.
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.
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.
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: 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.
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.
📚 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.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
📚 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
📚 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:
backend/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:
backend/apps/mentorship/api/internal/queries/program.py
📚 Learning: 2025-07-31T07:05:25.056Z
Learnt from: ahmedxgouda
Repo: OWASP/Nest PR: 0
File: :0-0
Timestamp: 2025-07-31T07:05:25.056Z
Learning: In the OWASP Nest project, Django views may not properly access authenticated users from sessions created by Strawberry GraphQL mutations. The issue occurs because Django's AuthenticationMiddleware doesn't always populate request.user from session data that GraphQL context successfully uses via info.context.request.user. The solution is to manually resolve users from session data using request.session.get('_auth_user_id') and User.objects.select_related('github_user').get(pk=user_id) to match the same authentication mechanism used by GraphQL context.
Applied to files:
backend/apps/mentorship/api/internal/queries/program.py
🧬 Code graph analysis (2)
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 (3)
backend/apps/mentorship/api/internal/queries/module.py (2)
10-10: Program import looks appropriate for status gating.
46-57:Module.published_modulescorrectly enforcesProgram.status == PUBLISHED.The
PublishedModuleManagerfilter inget_queryset()appliesprogram__status=Program.ProgramStatus.PUBLISHED, so this is a safe choice for public access queries. Good practice.backend/apps/mentorship/api/internal/queries/program.py (1)
23-39: Clarify whether COMPLETED programs should transition to private or be excluded from public access.The codebase currently treats COMPLETED as non-public (same as DRAFT). This is intentional and consistent—the
is_indexableproperty, module managers, and indexing all explicitly filter for PUBLISHED only, never including COMPLETED. However, this represents an important design decision: programs move DRAFT → PUBLISHED → COMPLETED → private/archived.If completed programs should remain publicly discoverable (e.g., archive browsing), the suggestion in the diff is correct. If they should be removed from public view after completion, the current implementation is correct and needs no change.
Confirm with the team whether the intended program lifecycle keeps COMPLETED programs public or treats them as archived/private.
There was a problem hiding this comment.
Actionable comments posted: 3
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)
95-133: ObjectDoesNotExist exceptions are not mapped to HTTP 404 responses; custom error handler configuration is required.The
get_modulefunction correctly enforces access control withObjectDoesNotExistexceptions, but the verification reveals that without a custom error handler in the strawberry schema, these exceptions return HTTP 200 OK with GraphQL errors (following the standard GraphQL spec) rather than HTTP 404 as intended.To satisfy the verification requirement ("ensure the GraphQL-to-HTTP layer treats ObjectDoesNotExist as notFound/404"), add a custom error handler to the strawberry schema or use a strawberry extension to map
ObjectDoesNotExistexceptions to HTTP 404 responses. This is essential to prevent data exposure about whether a resource exists or is inaccessible.
🤖 Fix all issues with AI agents
In @backend/apps/common/extensions.py:
- Around line 40-74: The _get_user_role method currently does DB hits despite
prefetching; replace the .filter().exists() calls with in-memory checks against
the prefetched relations: after loading mentor and program (using
Program.prefetch_related("admins", "modules__mentors")), check admin membership
with something like any(admin.id == mentor.id for admin in program.admins.all())
and check mentor membership in modules with any(any(m.id == mentor.id for m in
module.mentors.all()) for module in program.modules.all()), returning "admin" if
either is true and "public" otherwise.
In @backend/apps/mentorship/signals/program.py:
- Around line 26-38: The signal receiver program_post_save_clear_graphql_cache
currently has an incorrect first parameter `self`; update its signature to match
Django receivers by removing `self` so it reads def
program_post_save_clear_graphql_cache(sender, instance, **kwargs): and, if you
need to suppress the naming-lint, add the same "# noqa: N805" comment used on
the other receiver; keep the @receiver(post_save, sender=Program) decorator and
the existing body (logger call and
invalidate_program_graphql_cache(instance.key)).
🧹 Nitpick comments (2)
backend/apps/mentorship/api/internal/queries/module.py (1)
16-33: Harden + de-N+1_is_program_admin_or_mentor(possibleuser.github_userAttributeError; extra queries).
Current implementation can issue N queries (per module) and will crash if an authenticated user lacksgithub_user.Proposed refactor
-def _is_program_admin_or_mentor(user, program) -> bool: +def _is_program_admin_or_mentor(user, program: Program) -> bool: """Check if user is an admin or mentor of the program.""" - if not user or not user.is_authenticated: + if not getattr(user, "is_authenticated", False): return False + + github_user = getattr(user, "github_user", None) + if not github_user: + return False try: - mentor = Mentor.objects.get(github_user=user.github_user) + mentor = Mentor.objects.only("id").get(github_user=github_user) if program.admins.filter(id=mentor.id).exists(): return True - - for module in program.modules.all(): - if module.mentors.filter(id=mentor.id).exists(): - return True except Mentor.DoesNotExist: - pass - - return False + return False + + return program.modules.filter(mentors=mentor).exists()backend/apps/mentorship/api/internal/queries/program.py (1)
23-63: Refactor auth logic to eliminate N+1 queries and reduce duplication.The
get_programmethod loops through modules with per-moduleexists()checks, which defeats the prefetch optimization and mirrors the duplicate pattern inmodule.py's_is_program_admin_or_mentorhelper. Additionally,user.github_useris accessed without a null guard.Replace the loop with a single filtered query:
program.modules.filter(mentors=mentor).exists(). Also guardgithub_useraccess to prevent AttributeError if the attribute is missing.Consider extracting the shared "admin-or-mentor" authorization logic into a reusable helper to prevent further duplication across queries and mutations.
📜 Review details
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (7)
backend/apps/common/extensions.pybackend/apps/core/utils/index.pybackend/apps/mentorship/api/internal/queries/module.pybackend/apps/mentorship/api/internal/queries/program.pybackend/apps/mentorship/signals/program.pybackend/tests/apps/common/extensions_test.pyfrontend/src/app/mentorship/programs/page.tsx
🧰 Additional context used
🧠 Learnings (9)
📚 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.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.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.
Applied to files:
frontend/src/app/mentorship/programs/page.tsxbackend/apps/mentorship/api/internal/queries/program.py
📚 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/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/tests/apps/common/extensions_test.pybackend/apps/mentorship/signals/program.pybackend/apps/core/utils/index.pybackend/apps/mentorship/api/internal/queries/module.pybackend/apps/common/extensions.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/tests/apps/common/extensions_test.pybackend/apps/mentorship/signals/program.pybackend/apps/core/utils/index.pybackend/apps/mentorship/api/internal/queries/module.pybackend/apps/common/extensions.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/program.py
📚 Learning: 2025-07-31T07:05:25.056Z
Learnt from: ahmedxgouda
Repo: OWASP/Nest PR: 0
File: :0-0
Timestamp: 2025-07-31T07:05:25.056Z
Learning: In the OWASP Nest project, Django views may not properly access authenticated users from sessions created by Strawberry GraphQL mutations. The issue occurs because Django's AuthenticationMiddleware doesn't always populate request.user from session data that GraphQL context successfully uses via info.context.request.user. The solution is to manually resolve users from session data using request.session.get('_auth_user_id') and User.objects.select_related('github_user').get(pk=user_id) to match the same authentication mechanism used by GraphQL context.
Applied to files:
backend/apps/mentorship/api/internal/queries/program.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)
backend/tests/apps/common/extensions_test.py (1)
backend/apps/common/extensions.py (1)
resolve(97-117)
backend/apps/mentorship/signals/program.py (2)
backend/apps/core/utils/index.py (1)
invalidate_program_graphql_cache(273-309)backend/apps/mentorship/models/program.py (1)
Program(18-87)
backend/apps/mentorship/api/internal/queries/module.py (3)
backend/apps/mentorship/models/mentor.py (1)
Mentor(11-40)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/common/extensions.py (4)
backend/apps/mentorship/models/mentor.py (1)
Mentor(11-40)backend/apps/mentorship/api/internal/nodes/program.py (1)
admins(32-34)backend/apps/mentorship/api/internal/queries/mentorship.py (1)
is_mentor(36-48)backend/apps/mentorship/api/internal/nodes/module.py (1)
mentors(38-40)
backend/apps/mentorship/api/internal/queries/program.py (4)
backend/apps/mentorship/api/internal/nodes/program.py (2)
ProgramNode(15-34)admins(32-34)backend/apps/mentorship/models/program.py (2)
Program(18-87)ProgramStatus(25-28)backend/apps/mentorship/models/mentor.py (1)
Mentor(11-40)backend/apps/mentorship/api/internal/nodes/module.py (1)
mentors(38-40)
🔇 Additional comments (5)
backend/apps/core/utils/index.py (1)
273-309: LGTM! Cache invalidation logic is well-implemented.The cache key generation correctly mirrors the format in
backend/apps/common/extensions.py:generate_key(), ensuring proper invalidation. The function appropriately covers both GraphQL queries (getProgram, getProgramModules) and both roles (admin, public).backend/apps/common/extensions.py (1)
76-117: Role-aware caching implementation is sound.The updated caching logic correctly:
- Determines user role for fields in
ROLE_AWARE_FIELDS- Incorporates role into cache key generation
- Uses separate
cache.get()andcache.set()operations for explicit controlThe cache key format ensures proper isolation between admin and public views.
backend/tests/apps/common/extensions_test.py (1)
147-163: LGTM! Test updates correctly reflect caching implementation changes.The tests now properly validate the new caching behavior using separate
cache.get()andcache.set()operations instead ofcache.get_or_set(). The assertions correctly verify:
- Cache hits return cached data without calling the resolver
- Cache misses call the resolver and store the result
frontend/src/app/mentorship/programs/page.tsx (1)
36-39: Remove the client-side filtering—it's redundant.Unpublished programs are already excluded from the Algolia index at the backend level via:
ProgramIndex.get_entities()filters to only published programsProgramIndex.should_index = "is_indexable"prevents individual non-published records from being indexedThe frontend filter provides no additional security since Algolia only returns published programs. Removing lines 36–39 simplifies the code without sacrificing safety.
Likely an incorrect or invalid review comment.
backend/apps/mentorship/api/internal/queries/module.py (1)
82-92: Good:get_project_modulesnow usesModule.published_modulesfor public visibility.
This matches the objective of only exposing modules from published programs via public entry points.
mrkeshav-05
force-pushed
the
fix/hide-public-view
branch
2 times, most recently
from
2ad2f4e to
1ef1982
Compare
![cubic-dev-ai[bot]](https://avatars.githubusercontent.com/in/1082092?s=60&v=4){kind=small}
There was a problem hiding this comment.
🧹 Nitpick comments (1)
backend/tests/apps/mentorship/utils_test.py (1)
17-42: Consider extracting shared info fixtures to reduce duplication.The
mock_infoandmock_anonymous_infofixtures are duplicated nearly identically across three test files (utils_test.py,api_queries_program_test.py,api_queries_module_test.py). Extracting them into a sharedconftest.pywould reduce maintenance burden.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@backend/tests/apps/mentorship/utils_test.py` around lines 17 - 42, Extract the duplicated fixtures mock_authenticated_info and mock_anonymous_info into a shared test fixture file (conftest.py): define the two fixtures there using MagicMock with spec=GithubUser and spec=strawberry.Info, setting request.user, is_authenticated, github_user, and return mock_info as shown; then remove the duplicate definitions from backend/tests/apps/mentorship/utils_test.py, backend/tests/.../api_queries_program_test.py, and backend/tests/.../api_queries_module_test.py so tests import the shared fixtures automatically; ensure the fixture names (mock_authenticated_info, mock_anonymous_info) and their behavior remain identical to avoid breaking tests.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Nitpick comments:
In `@backend/tests/apps/mentorship/utils_test.py`:
- Around line 17-42: Extract the duplicated fixtures mock_authenticated_info and
mock_anonymous_info into a shared test fixture file (conftest.py): define the
two fixtures there using MagicMock with spec=GithubUser and
spec=strawberry.Info, setting request.user, is_authenticated, github_user, and
return mock_info as shown; then remove the duplicate definitions from
backend/tests/apps/mentorship/utils_test.py,
backend/tests/.../api_queries_program_test.py, and
backend/tests/.../api_queries_module_test.py so tests import the shared fixtures
automatically; ensure the fixture names (mock_authenticated_info,
mock_anonymous_info) and their behavior remain identical to avoid breaking
tests.
|
Hi @kasya , The PR is ready for review with the new changes. Uploading Screen Recording 2026-02-22 at 1.40.25 AM.mov… |
kasya
left a comment
kasya
left a comment
There was a problem hiding this comment.
@mrkeshav-05 thanks for working on this!
This works great.👍🏼
Left a few comments about implementation ⬇️
backend/apps/mentorship/utils.py
Outdated
| def has_program_access(info: strawberry.Info, program: Program) -> bool: | ||
| """Check if the current user has admin or mentor access to a program. | ||
|
|
||
| Returns True if the user is authenticated and is either an admin or | ||
| a mentor of the given program, False otherwise. | ||
| """ | ||
| user = info.context.request.user | ||
| if not user.is_authenticated: | ||
| return False | ||
|
|
||
| is_admin = program.admins.filter(nest_user=user).exists() | ||
| if is_admin: | ||
| return True | ||
|
|
||
| if user.github_user: | ||
| is_mentor = program.modules.filter(mentors__github_user=user.github_user).exists() | ||
| if is_mentor: | ||
| return True | ||
|
|
||
| return False | ||
|
|
There was a problem hiding this comment.
Is this file the best place for this? 🤔
Maybe we could have it on the Program class instead (with probably renaming it to something like user_has_access())?
backend/apps/mentorship/utils.py
Outdated
| is_admin = program.admins.filter(nest_user=user).exists() | ||
| if is_admin: | ||
| return True | ||
|
|
||
| if user.github_user: | ||
| is_mentor = program.modules.filter(mentors__github_user=user.github_user).exists() | ||
| if is_mentor: | ||
| return True | ||
|
|
||
| return False |
There was a problem hiding this comment.
This could be simplified:
| is_admin = program.admins.filter(nest_user=user).exists() | |
| if is_admin: | |
| return True | |
| if user.github_user: | |
| is_mentor = program.modules.filter(mentors__github_user=user.github_user).exists() | |
| if is_mentor: | |
| return True | |
| return False | |
| return program.admins.filter(nest_user=user).exists() or ( | |
| user.github_user | |
| and program.modules.filter(mentors__github_user=user.github_user).exists() | |
| ) |
| @@ -28,17 +39,22 @@ def get_program_modules(self, program_key: str) -> list[ModuleNode]: | |||
| def get_project_modules(self, project_key: str) -> list[ModuleNode]: | |||
There was a problem hiding this comment.
I don't think we use this anywhere 🤔 Could you check if this is redundant and if so - clean this up?
There was a problem hiding this comment.
Yes, I checked and this function isn’t being used by any frontend component. I’ll go ahead and clean it up.
mrkeshav-05
dismissed stale reviews from coderabbitai[bot] and cubic-dev-ai[bot]
via
9eece96
|
Hey @kasya, the PR is ready for review. Please let me know if any changes are needed. |
|
kasya
left a comment
kasya
left a comment
There was a problem hiding this comment.
@mrkeshav-05 Hi! Thanks for making those updates.
This works great 👍🏼
Codecov Report✅ All modified and coverable lines are covered by tests. Additional details and impacted files@@ Coverage Diff @@
## main #3117 +/- ##
=======================================
Coverage 99.75% 99.75%
=======================================
Files 519 519
Lines 16068 16080 +12
Branches 2192 2196 +4
=======================================
+ Hits 16029 16041 +12
Misses 30 30
Partials 9 9
Flags with carried forward coverage won't be shown. Click here to find out more.
Continue to review full report in Codecov by Sentry.
🚀 New features to boost your workflow:
|
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