Skip to content
Merged
Show file tree
Hide file tree
Changes from 1 commit
Commits
File filter

Filter by extension

Filter by extension


Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
6 changes: 5 additions & 1 deletion .eslintrc.json
Original file line number Diff line number Diff line change
Expand Up @@ -18,9 +18,13 @@
"rules": {
"@typescript-eslint/no-empty-function": "off",
"@typescript-eslint/no-empty-interface": "off",
"@typescript-eslint/no-explicit-any": "warn",
"@typescript-eslint/no-namespace": "off",
"@typescript-eslint/no-non-null-assertion": "off",
"@typescript-eslint/no-unused-vars": "warn",
"@typescript-eslint/no-unused-vars": [
"warn",
{ "varsIgnorePattern": "^_" }
],
"no-console": ["warn", { "allow": ["warn", "error"] }],
"no-empty": "off",
"no-inner-declarations": "off",
Expand Down
674 changes: 361 additions & 313 deletions package-lock.json

Large diffs are not rendered by default.

4 changes: 2 additions & 2 deletions package.json
Original file line number Diff line number Diff line change
Expand Up @@ -21,10 +21,10 @@
"eslint-plugin-n": "^16.0.2",
"eslint-plugin-promise": "^6.1.1",
"eslint-plugin-solid": "^0.13.0",
"laravel-vite-plugin": "^0.7.8",
"laravel-vite-plugin": "^0.8.1",
"postcss": "^8.4.29",
"prettier": "^3.0.3",
"sass": "^1.66.1",
"sass": "^1.69.0",
"tailwindcss": "^3.3.3",
"typescript": "^5.2.2",
"vite": "^4.4.9",
Expand Down
3 changes: 2 additions & 1 deletion resources/js/App.tsx
Original file line number Diff line number Diff line change
@@ -1,4 +1,4 @@
import {Outlet, Route, Routes, useParams} from "@solidjs/router";
import {Navigate, Outlet, Route, Routes, useParams} from "@solidjs/router";
import {createQuery} from "@tanstack/solid-query";
import {AccessBarrier} from "components/utils";
import {System} from "data-access/memo-api";
Expand Down Expand Up @@ -31,6 +31,7 @@ const App: VoidComponent = () => {
component={RootPageWithFacility}
>
<UnknownNotFound />
<Route path="/" element={<Navigate href="home" />} />
<Route path="/home" component={NotYetImplemented} />
<Route path="/meetings" component={NotYetImplemented} />
<Route path="/" component={FacilityStaffPages}>
Expand Down
12 changes: 6 additions & 6 deletions resources/js/components/ui/Table/TQueryTable.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -80,11 +80,11 @@ export interface TQueryTableProps {
* A list of columns that are always included in the request (and available on the row objects),
* even if they are not visible.
*/
intrinsicColumns?: string[];
intrinsicColumns?: readonly string[];
/** Additional column names. Their options are taken from `columnOptions`. */
additionalColumns?: string[];
additionalColumns?: readonly string[];
/** Overrides for the definition of specific columns. */
columnOptions?: Partial<Record<string, ColumnOptions>>;
columnOptions?: Readonly<Partial<Record<string, ColumnOptions>>>;
/**
* The ordering of the columns. All the columns present on the backend and not present
* in this list are placed at the end.
Expand All @@ -94,12 +94,12 @@ export interface TQueryTableProps {
*
* In the current implementation the order of columns cannot be changed.
*/
initialColumnsOrder?: string[];
initialVisibleColumns?: string[];
initialColumnsOrder?: readonly string[];
initialVisibleColumns?: readonly string[];
initialSort?: SortingState;
initialPageSize?: number;
/** These columns are ignored and never shown. */
ignoreColumns?: string[];
ignoreColumns?: readonly string[];
/** Element to put below table, after the summary. */
customSectionBelowTable?: JSX.Element;
}
Expand Down
28 changes: 22 additions & 6 deletions resources/js/components/utils/AccessBarrier.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -27,6 +27,9 @@ export interface AccessBarrierProps extends Pick<QueryBarrierProps, "Error" | "P
facilityUrl?: string;
}

/** The roles for which querying facility permissions is necessary. */
const FACILITY_ROLES = new Set<PermissionKey>(["facilityMember", "facilityClient", "facilityStaff", "facilityAdmin"]);

/**
* Utility component that checks authentication
* state and user's permissions
Expand All @@ -53,12 +56,25 @@ export const AccessBarrier: ParentComponent<AccessBarrierProps> = (props) => {
);
const [queryBarrierProps, localProps] = splitProps(merged, ["Error", "Pending"]);
const facilitiesQuery = createQuery(System.facilitiesQueryOptions);
const facilityId = () => facilitiesQuery.data?.find(({url}) => url === localProps.facilityUrl)?.id;

const statusQuery = createQuery(() => User.statusQueryOptions(facilityId()));
const accessGranted = () =>
statusQuery.isSuccess && localProps.roles?.every((role) => statusQuery.data?.permissions[role]);

const statusQuery = createQuery(() => {
// Only load the facility permissions if they are actually checked.
if (localProps.roles.some((role) => FACILITY_ROLES.has(role))) {
const facilityId = facilitiesQuery.data?.find(({url}) => url === localProps.facilityUrl)?.id;
if (facilityId) {
return User.statusWithFacilityPermissionsQueryOptions(facilityId);
}
// If the facility is not available, return statusQueryOptions below, which will fail
Copy link
Contributor

Choose a reason for hiding this comment

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

Nie da się uniknąć niepotrzebnych zapytań, jeśli wiadomo że "fail anyway"?

Też nie widać gdzie i czemu "fail anyway", bo pod spodem nie widzę sprawdzania "facility permissions fields".

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

Facility permission fields są w localProps.roles. Więc nigdy nie będzie accessGranted skoro ich nie ma w wyniku (są wszystkie false).

Zwracam disabled query.

Copy link
Contributor

Choose a reason for hiding this comment

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

To chyba nadal nie rozumiem, "Return a disabled query, which will be shown as pending"? Pending forever? Dlaczego nie jakiś błąd w takim razie?

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

Brak facility jest tutaj stanem przejściowym. Jeżeli wynika z tego że to facility w ogóle nie istnieje no to router pokaże jakąś stronę że not found. Więc tutaj to może wynikać z tego, że facilities się jeszcze nie załadowały, no więc pending. W ogóle to jest przypadek którego mi się w praktyce nie udało nawet uzyskać i próbuję go zrobić bez zbędnego kombinowania i możliwie szybko :) Naprawdę nie warto w to rozwiązanie inwestować moim zdaniem.

Copy link
Contributor

Choose a reason for hiding this comment

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

No dobra, ale czyż nie jest najprościej jakieś throw new Error() w takim dziwnym przypadku?

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

No nie, bo jednak teoretycznie się to może zdarzyć, jeśli na przykład facilities nie są dostępne, bo się jeszcze ładują, czy coś takiego. To nie jest jakiś twardy error, tylko raczej takie poczekamy, zobaczymy, a póki co nie masz dostępu.

Copy link
Contributor

Choose a reason for hiding this comment

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

Ok, po rozmowie na discord lepiej rozumiem, że to komplikacje solidowe z sekwencyjnym query. Może by tu pomogło zrozumieć coś w stylu "// Return disabled query to wait for facilitiesQuery.data"

No i nadal error facilitiesQuery nie jest obsługiwany poprawnie (zawiśnie na loaderze)? Jak nie ma lepszych pomysłów, to albo dwa QueryBarrier albo przynajmniej poproszę TODO albo komentarz że tak jest i trudno bo np. zadowalamy się toastem na takim zawiśniętym loaderze.

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

Dodałem podwójne QueryBarrier i wydaje się działać, chociaż może dobrze gdybyś to przeklikał.

// the permissions check anyway because the facility permissions fields are false in it.
}
return User.statusQueryOptions();
});
const accessGranted = () => {
if (!statusQuery.isSuccess) {
return false;
}
const permissions = statusQuery.data!.permissions as Partial<Record<PermissionKey, boolean>>;
return localProps.roles?.every((role) => permissions[role]);
};
return (
<QueryBarrier queries={[statusQuery]} {...queryBarrierProps}>
<Show when={accessGranted()} fallback={<localProps.Fallback />}>
Expand Down
45 changes: 31 additions & 14 deletions resources/js/components/utils/InitializeTanstackQuery.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -15,12 +15,15 @@ import toast from "solid-toast";
import {cx, useLangFunc} from ".";
import {MemoLoader} from "../ui";

/** A list of HTTP response status codes for which a toast should not be displayed. */
type QuietHTTPStatuses = number[];

declare module "@tanstack/query-core" {
interface QueryMeta {
quietError?: boolean;
quietHTTPStatuses?: QuietHTTPStatuses;
}
interface MutationMeta {
quietError?: boolean;
quietHTTPStatuses?: QuietHTTPStatuses;
isFormSubmit?: boolean;
}
}
Expand All @@ -34,25 +37,37 @@ export const InitializeTanstackQuery: ParentComponent = (props) => {
const t = useLangFunc();
function toastErrors(error: Error, meta?: Partial<QueryMeta & MutationMeta>) {
if (!isAxiosError<Api.ErrorResponse>(error)) return;
if ((error?.status && error.status >= 500) || !meta?.quietError) {
const status = error.response?.status;
if (!status || !meta?.quietHTTPStatuses?.includes(status)) {
let errors = error.response?.data.errors;
if (meta?.isFormSubmit) {
// Validation errors will be handled by the form.
errors = errors?.filter((e) => !Api.isValidationError(e));
}
if (errors?.length) {
const errorMessages = errors.map((e) =>
t(e.code, {
const errorMessages = errors.map((e) => {
const params = {
...(Api.isValidationError(e) ? {attribute: e.field} : undefined),
...e.data,
}),
);
for (const msg of errorMessages) {
console.warn(`Error toast shown: ${msg}`);
}
};
const translated = () => t(e.code, params);
function logWhenAvailable(first = false) {
const text = translated();
if (text) {
console.warn(`Error toast shown: ${text}`);
} else {
if (first)
console.warn(`Error toast shown (translations not ready): ${e.code} ${JSON.stringify(params)}`);
// We're not in reactive scope, so use timeout to wait until the translations are available.
setTimeout(logWhenAvailable, 500);
}
}
logWhenAvailable(true);
return translated;
});
toast.error(() => (
<ul class={cx({"list-disc pl-6": errorMessages.length > 1})} style={{"overflow-wrap": "anywhere"}}>
<For each={errorMessages}>{(msg) => <li>{msg}</li>}</For>
<For each={errorMessages}>{(msg) => <li>{msg()}</li>}</For>
</ul>
));
}
Expand All @@ -64,10 +79,12 @@ export const InitializeTanstackQuery: ParentComponent = (props) => {
defaultOptions: {
queries: {
refetchOnReconnect: true,
refetchOnMount: false,
refetchOnWindowFocus: false,
// When opening a page, reload data if it's older than a couple of seconds.
staleTime: 5 * 1000,
retry: false,
retryOnMount: false,
// This is very important. The default reconcile algorithm somehow breaks the data and
// reactivity in complicated ways. This line is basically `broken: false`.
reconcile: false,
},
},
queryCache: new QueryCache({
Expand Down
4 changes: 2 additions & 2 deletions resources/js/data-access/memo-api/groups/Admin.ts
Original file line number Diff line number Diff line change
Expand Up @@ -28,9 +28,9 @@ export namespace Admin {

const getUsersListBase = (request?: Api.Request.GetListParams, config?: Api.Config) =>
V1.get<Api.Response.GetList<AdminUserResource>>("/admin/user/list", {...config, params: request});
export const getUsersList = (request?: Api.Request.GetListParams, config?: Api.Config) =>
const getUsersList = (request?: Api.Request.GetListParams, config?: Api.Config) =>
getUsersListBase(request, config).then(parseGetListResponse);
export const getUser = createGetFromList(getUsersListBase);
const getUser = createGetFromList(getUsersListBase);

export const createMember = (member: Api.Request.Create<MemberResource>, config?: Api.Config) =>
V1.post("/admin/member", member, config);
Expand Down
4 changes: 3 additions & 1 deletion resources/js/data-access/memo-api/groups/System.ts
Original file line number Diff line number Diff line change
Expand Up @@ -9,7 +9,7 @@ import {parseGetListResponse} from "../utils";
* @see {@link http://localhost:9081/api/documentation#/System local docs}
*/
export namespace System {
export const getFacilitiesList = (config?: Api.Config) =>
const getFacilitiesList = (config?: Api.Config) =>
V1.get<Api.Response.GetList<FacilityResource>>("/system/facility/list", config).then(parseGetListResponse);

export const keys = {
Expand All @@ -23,6 +23,8 @@ export namespace System {
({
queryFn: ({signal}) => getFacilitiesList({signal}),
queryKey: keys.facilityList(),
// Prevent refetching on every page.
refetchOnMount: false,
}) satisfies SolidQueryOptions;

export function useInvalidator() {
Expand Down
40 changes: 35 additions & 5 deletions resources/js/data-access/memo-api/groups/User.ts
Original file line number Diff line number Diff line change
Expand Up @@ -10,7 +10,7 @@ import {parseGetResponse} from "../utils";
* @see {@link http://localhost:9081/api/documentation#/User local docs}
*/
export namespace User {
export const getStatus = (facilityId?: string, config?: Api.Config) =>
const getStatus = (facilityId?: string, config?: Api.Config) =>
V1.get<Api.Response.Get<GetStatusData>>(facilityId ? `/user/status/${facilityId}` : "/user/status", config).then(
parseGetResponse,
);
Expand Down Expand Up @@ -42,19 +42,49 @@ export namespace User {

export const keys = {
all: () => ["user"] as const,
status: (facilityId?: string) => [...keys.all(), "status", facilityId] as const,
statusAll: () => [...keys.all(), "status"] as const,
status: (facilityId?: string) => [...keys.statusAll(), facilityId] as const,
};

export const statusQueryOptions = (facilityId?: string) =>
type PermissionsFacilityKeys = "facilityId" | "facilityMember" | "facilityClient" | "facilityStaff" | "facilityAdmin";
// Ensure these are really keys.
type _FacilityPermissions = Pick<PermissionsResource, PermissionsFacilityKeys>;

export type GetStatusWithoutFacilityData = {
user: UserResource;
permissions: Omit<PermissionsResource, PermissionsFacilityKeys>;
members: MemberResource[];
};

const STATUS_QUERY_OPTIONS = {
// Prevent refetching on every page.
refetchOnMount: false,
/** Prevent displaying toast when user is not logged in - the login page will be displayed. */
meta: {quietHTTPStatuses: [401]},
};

/** Query options for user status, without facility permissions. */
export const statusQueryOptions = () =>
({
// As a possible optimisation, this query could try to reuse any query with facility permissions,
// that happens to be active.
queryFn: ({signal}): Promise<GetStatusWithoutFacilityData> => getStatus(undefined, {signal}),
queryKey: keys.status(),
...STATUS_QUERY_OPTIONS,
}) satisfies SolidQueryOptions<GetStatusWithoutFacilityData>;

/** Query options for user status with facility permissions. */
export const statusWithFacilityPermissionsQueryOptions = (facilityId: string) =>
({
queryFn: ({signal}) => getStatus(facilityId, {signal}),
queryKey: keys.status(facilityId),
}) satisfies SolidQueryOptions;
...STATUS_QUERY_OPTIONS,
}) satisfies SolidQueryOptions<GetStatusData>;

export function useInvalidator() {
const queryClient = useQueryClient();
return {
status: () => queryClient.invalidateQueries({queryKey: keys.status()}),
statusAndFacilityPermissions: () => queryClient.invalidateQueries({queryKey: keys.statusAll()}),
};
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -26,7 +26,7 @@ export type AdminUserResource = {
* @type {string(uuid)}
* @example '67da972b-34d7-4f89-b8ae-322d96b4954d'
*/
lastLoginFacilityId: string;
lastLoginFacilityId: string | null;
passwordExpireAt: string | null;
hasPassword: boolean;
createdAt: string;
Expand Down
Loading