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
anytypes anywhere. Useunknownwith 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,successare properly typed asRef<boolean>/Ref<string>. - [ ] API error handling uses
FetchErrorfromofetchfor type-safe access to error payloads. - [ ] Template refs are typed (e.g.,
ref<HTMLFormElement | null>(null)). - [ ]
constis 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). - [ ]
UiButtonuses the appropriatevariantandsizeprops -- not custom one-off button styles. - [ ] Forms use
@submit.preventon the<form>element, not@clickon 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$fetchoruseApiFetchin components. - [ ] Service methods catch errors, report to
Sentry.captureException(), and re-throw with a clean message. - [ ] The service has a
getErrorMessagehelper that safely extracts the message fromFetchError. - [ ] For standard CRUD, the service extends
BasicCrudServiceor 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
useFormValidationwithuseValidationRuleshelpers -- 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
reactivewith the appropriate partial domain type. - [ ] Loading state is shown on the submit button via
:loading. - [ ] Success feedback uses
useSnackBar().show(). - [ ] The form emits
closeafter 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'indefinePageMeta. - [ ] 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/successpattern withtry/catch/finally. - [ ] Errors are reset before the operation (
error.value = '';). - [ ] Loading is reset in
finallyto 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(notref) to avoid deep reactivity on component definitions. - [ ]
watchcallbacks that trigger API calls have debouncing where appropriate (search inputs useuseListFiltersbuilt-in debounce). - [ ] Heavy components inside modals are conditionally rendered with
v-if, not hidden withv-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
styleattributes 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-4patterns 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, notTable.vue). - [ ] UI components use the
Uiprefix (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, orshowprefix where appropriate. - [ ] Constants use
UPPER_SNAKE_CASE. - [ ] Page directories use kebab-case (
surgery-locations,stock-movement).
List Pages
- [ ] Uses
useListFiltersfor pagination, search debounce, and URL query sync. - [ ] Calls
filter()on mount viaonMounted. - [ ] Calls
refetchCurrentPage()after create/update/delete mutations. - [ ] Non-search filters (status, date) use
watchImmediateFilterfor instant updates. - [ ] Pagination is handled by
setPage(), not manual query manipulation.
