Skip to content

Last updated:

Code Review Guidelines

Use this checklist when reviewing pull requests in the CPR frontend. Each section targets a specific area of concern. A PR should satisfy all applicable sections before merging.

TypeScript

  • [ ] No any types anywhere. Use unknown with type narrowing, or the correct specific type.
  • [ ] Props are defined with defineProps<Props>() using a TypeScript interface.
  • [ ] Emits are defined with defineEmits<{ ... }>() with full type signatures.
  • [ ] Composable return types are inferable -- loading, error, success are properly typed as Ref<boolean> / Ref<string>.
  • [ ] API error handling uses FetchError from ofetch for type-safe access to error payloads.
  • [ ] Template refs are typed (e.g., ref<HTMLFormElement | null>(null)).
  • [ ] const is used for all bindings that are not reassigned.
ts
// What to look for in error handling:
// good
catch (err: unknown) {
  const fetchError = err as FetchError<{ message?: string }>;
  const message = fetchError.data?.message ?? fetchError.message ?? 'Something went wrong';
}

// reject
catch (err: any) {
  console.log(err.message);
}

Components

  • [ ] Uses <script setup lang="ts"> -- no Options API.
  • [ ] Single responsibility -- the component does one thing well.
  • [ ] Component size is reasonable (~150 lines of template, ~100 lines of script max). Logic is extracted to composables when it grows.
  • [ ] Slots are used where child content varies. Named slots are used for complex layouts (e.g., #actions, #subtitle).
  • [ ] UiButton uses the appropriate variant and size props -- not custom one-off button styles.
  • [ ] Forms use @submit.prevent on the <form> element, not @click on the submit button.
vue
<!-- good: proper form submission -->
<form @submit.prevent="handleSubmit">
  <UiButton variant="primary" type="submit" :loading="loading">
    Add Doctor
  </UiButton>
</form>

API Calls

  • [ ] All API calls go through a service class (e.g., doctorService.getDoctors()). No raw $fetch or useApiFetch in components.
  • [ ] Service methods catch errors, report to Sentry.captureException(), and re-throw with a clean message.
  • [ ] The service has a getErrorMessage helper that safely extracts the message from FetchError.
  • [ ] For standard CRUD, the service extends BasicCrudService or follows the same pattern.
  • [ ] Service singleton is exported as both named and default export.
ts
// Verify the service follows this pattern:
class DoctorService {
  private getErrorMessage(err: unknown): string {
    const fetchError = err as FetchError<unknown>;
    return (
      (fetchError.data as { message?: string })?.message ??
      fetchError.message ??
      'Something went wrong'
    );
  }

  async getDoctor(id: number): Promise<Doctor> {
    try {
      const res: ApiResponse<Doctor> = await useApiFetch(`/doctors/${id}`, {
        method: 'GET',
      });
      return res.data;
    } catch (err: unknown) {
      const message = this.getErrorMessage(err);
      Sentry.captureException(err);
      throw new Error(message);
    }
  }
}

export const doctorService = new DoctorService();
export default doctorService;

State Management

  • [ ] Pinia stores are used for state shared across multiple components (list data, pagination, auth).
  • [ ] Stores contain only reactive state and simple getters -- no API calls or business logic.
  • [ ] Composables populate stores, not the other way around.
  • [ ] Module-level shared state (like useDialog, useSnackBar) is only used for true singletons.
  • [ ] Local component state stays local unless there is a clear reason to lift it.
ts
// Stores should look like this -- data only:
export const useDoctorStore = defineStore('doctor', () => {
  const list = ref<Doctor[]>([]);
  const pagination = ref<APIPagination>({ ... });
  return { list, pagination };
});

Forms

  • [ ] Uses useFormValidation with useValidationRules helpers -- no ad-hoc validation logic.
  • [ ] Validation runs before the API call (if (!validate(form)) return;).
  • [ ] Error messages are bound to inputs via :error="errors.field_name".
  • [ ] Form state uses reactive with the appropriate partial domain type.
  • [ ] Loading state is shown on the submit button via :loading.
  • [ ] Success feedback uses useSnackBar().show().
  • [ ] The form emits close after successful submission.
ts
// Verify the form follows this pattern:
const { errors, validate } = useFormValidation<Partial<Doctor>>({
  first_name: [required('First Name')],
  last_name: [required('Last Name')],
  doctor_role_id: [required('Specialty')],
});

const handleSubmit = async () => {
  if (!validate(form)) return;
  await createDoctor(form);

  if (success.value) {
    snackbar.show('Doctor created successfully');
    emit('close');
  }
};

Permissions

  • [ ] UI elements for privileged actions are guarded with v-if="$can('permission.name')".
  • [ ] Page-level protection uses middleware: 'auth' in definePageMeta.
  • [ ] Destructive actions (delete, status change) check permissions before rendering the trigger.
  • [ ] Role-based sections use $hasRole() or $canAny() as appropriate.
vue
<!-- Verify create/edit/delete buttons are guarded: -->
<UiButton
  v-if="$can('doctors.create')"
  variant="primary"
  :left-icon="PlusIcon"
  @click="handleAddDoctorClick"
>
  Add Doctor
</UiButton>

Error Handling

  • [ ] All async composable functions follow the loading / error / success pattern with try/catch/finally.
  • [ ] Errors are reset before the operation (error.value = '';).
  • [ ] Loading is reset in finally to guarantee cleanup.
  • [ ] User-facing errors show a snackbar or inline error message -- errors are not silently swallowed.
  • [ ] Service-layer errors are captured with Sentry.captureException(err) before re-throwing.
ts
// Verify this structure in every async composable:
const action = async () => {
  loading.value = true;
  success.value = false;
  error.value = '';

  try {
    // ... API call
    success.value = true;
  } catch (err) {
    const message = err instanceof Error ? err.message : 'Something went wrong';
    error.value = message;
    success.value = false;
  } finally {
    loading.value = false;
  }
};

Performance

  • [ ] Derived values use computed, not methods that recalculate on every render.
  • [ ] Large lists use pagination via useListFilters -- no client-side filtering of unbounded data.
  • [ ] Dynamic dialog components use shallowRef (not ref) to avoid deep reactivity on component definitions.
  • [ ] watch callbacks that trigger API calls have debouncing where appropriate (search inputs use useListFilters built-in debounce).
  • [ ] Heavy components inside modals are conditionally rendered with v-if, not hidden with v-show.
ts
// computed for derived values
const doctorRoles = computed(() => enums.doctor_roles || []);

// shallowRef for component references
const component = shallowRef<Component | null>(null);

Styling

  • [ ] Uses Tailwind utility classes -- no inline style attributes or custom CSS for one-off layouts.
  • [ ] Responsive breakpoints are included where appropriate (grid-cols-1 md:grid-cols-2).
  • [ ] Consistent spacing and padding (uses px-6 py-4 patterns from the design system).
  • [ ] Colors match the project palette (primary: #117ea7, not arbitrary blue values).
  • [ ] Interactive states are handled (hover:, active:, disabled:, focus:).
  • [ ] Empty states have a visual treatment (icon + message, not just blank space).
vue
<!-- Good empty state: -->
<tr v-else>
  <td colspan="4" class="px-6 py-12 text-center text-gray-500">
    <div class="flex flex-col items-center justify-center gap-2">
      <ArchiveBoxIcon class="w-8 h-8 text-gray-300" />
      <p>No doctors found</p>
    </div>
  </td>
</tr>

Naming

  • [ ] Component files are PascalCase with domain prefix (e.g., DoctorTable.vue, not Table.vue).
  • [ ] UI components use the Ui prefix (e.g., UiButton.vue).
  • [ ] Composables follow use[Domain][Action].ts (e.g., useCreateDoctor.ts).
  • [ ] Stores are [domain].store.ts, services are [domain].service.ts, types are [domain].type.ts.
  • [ ] Events use kebab-case (page-change, row-click, update:modelValue).
  • [ ] Boolean refs/props use is, has, or show prefix where appropriate.
  • [ ] Constants use UPPER_SNAKE_CASE.
  • [ ] Page directories use kebab-case (surgery-locations, stock-movement).

List Pages

  • [ ] Uses useListFilters for pagination, search debounce, and URL query sync.
  • [ ] Calls filter() on mount via onMounted.
  • [ ] Calls refetchCurrentPage() after create/update/delete mutations.
  • [ ] Non-search filters (status, date) use watchImmediateFilter for instant updates.
  • [ ] Pagination is handled by setPage(), not manual query manipulation.

CPR - Clinical Patient Records