Skip to content

feat: nullable levels#188

Merged
theosiemensrhodes merged 3 commits intomainfrom
feature/optional-levels
Mar 7, 2026
Merged

feat: nullable levels#188
theosiemensrhodes merged 3 commits intomainfrom
feature/optional-levels

Conversation

@edlng
Copy link
Member

@edlng edlng commented Mar 4, 2026

Closes #176

Makes it so that non-exercise classes have nullable levels (i.e., doesn't show the level slider).

Signed-off-by: edlng <edwardliangedli@gmail.com>
@greptile-apps
Copy link
Contributor

greptile-apps bot commented Mar 4, 2026

Greptile Summary

This PR successfully makes lowerLevel and upperLevel nullable for non-exercise classes across the full stack: DB schema, Drizzle ORM, domain models, API validation, and UI components. The core feature is well-implemented with correct conditional rendering and form auto-clearing logic.

One actionable finding: The frontend ClassEditSchema enforces that exercise classes must provide levels, but this business rule is not enforced at the API schema layer. Direct API calls can bypass the UI and create exercise classes with null levels, potentially leading to inconsistent data.

Confidence Score: 4/5

  • Safe to merge; the feature works correctly end-to-end with one actionable validation gap at the API layer.
  • This PR implements the nullable levels feature comprehensively across DB, models, API schemas, and UI. The core functionality is sound with proper null guards and conditional rendering. The identified issue—missing server-side validation for the exercise category constraint—is a real business rule gap that could allow inconsistent data via direct API calls, but it doesn't block the feature from working correctly through the UI.
  • src/models/api/class.ts requires a server-side refinement to enforce that exercise categories must have levels (mirroring the frontend validation).

Last reviewed commit: a7a5279

@greptile-apps
Copy link
Contributor

greptile-apps bot commented Mar 4, 2026

Additional Comments (1)

src/models/api/class.ts, line 20
The frontend ClassEditSchema enforces that exercise classes must provide a levelRange (lines 29-40 in schema.ts). However, the CreateClass API schema (and UpdateClass) only mark the fields as nullable without any cross-field check. This means any caller that bypasses the UI—such as a direct API call or a future script—can create an exercise class with lowerLevel: null, upperLevel: null, and the server will accept it.

Consider adding a server-side refinement to ensure the business rule is enforced end-to-end:

export const CreateClass = z
  .object({
    termId: z.uuid(),
    image: z.string().optional(),
    name: z.string().nonempty(),
    description: z.string().optional(),
    meetingURL: z.url().optional(),
    lowerLevel: z.int().min(1).max(4).nullish(),
    upperLevel: z.int().min(1).max(4).nullish(),
    category: z.string(),
    subcategory: z.string().optional(),
    schedules: z.array(CreateSchedule).default([]),
  })
  .refine(
    (val) => {
      if (val.category.includes("Exercise")) {
        return val.lowerLevel != null && val.upperLevel != null;
      }
      return true;
    },
    {
      error: "Levels are required for exercise classes",
      path: ["lowerLevel"],
    },
  );

check(
"chk_lower_level_bounds",
sql`${table.lowerLevel} >= 1 AND ${table.lowerLevel} <= 4`,
sql`${table.lowerLevel} IS NULL OR (${table.lowerLevel} >= 1 AND ${table.lowerLevel} <= 4)`,
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

We should make the logic be either both lower and upper are present or the both are not present. Right now I believe the logic is not exactly that.

Signed-off-by: edlng <edwardliangedli@gmail.com>
@edlng edlng requested a review from theosiemensrhodes March 7, 2026 19:05
@theosiemensrhodes theosiemensrhodes merged commit 871405f into main Mar 7, 2026
theosiemensrhodes added a commit that referenced this pull request Mar 7, 2026
* main:
  chore: upgrade deps
  feat: nullable levels (#188)
theosiemensrhodes added a commit that referenced this pull request Mar 9, 2026
* main:
  chore: upgrade deps
  feat: nullable levels (#188)
  fix: remove One-on-One Exercise
  132 security dialog (#155)
  implement volunteer scoped availability settings content (#187)

# Conflicts:
#	pnpm-lock.yaml
#	src/components/classes/constants.ts
#	src/components/classes/edit/content/class-general-section.tsx
#	src/components/classes/edit/schema.ts
#	src/models/api/class.ts
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Make levels optional feature

2 participants