Skip to content

refactor: extract validation and error handling from RegisterForm #38371

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Closed
sharanyamahajan wants to merge 16 commits into from
Closed

refactor: extract validation and error handling from RegisterForm #38371

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
16 commits
Select commit Hold shift + click to select a range
1bba949
fix(sidebar): avoid sparse arrays when unregistering sidebar items
sharanyamahajan Jan 10, 2026
1d2d5ad
feat(ui): add skeleton loader to room members contextual bar
sharanyamahajan Jan 13, 2026
3d88da3
Merge branch 'develop' into feat/contextualbar-skeleton-loader
sharanyamahajan Jan 13, 2026
e024740
fix: prevent silent rollback failure in findOneAndDelete
sharanyamahajan Jan 14, 2026
4a77384
Merge pull request #1 from sharanyamahajan/fix-admin-home-mobile-card
sharanyamahajan Jan 15, 2026
7965766
Merge pull request #2 from sharanyamahajan/feat/contextualbar-skeleto…
sharanyamahajan Jan 16, 2026
396febe
fix: handle invalid JSON in MONGO_OPTIONS safely
sharanyamahajan Jan 17, 2026
04a703b
Merge pull request #3 from sharanyamahajan/fix/handle-invalid-mongo-o…
sharanyamahajan Jan 17, 2026
1d3fc02
refactor: remove setState from componentDidMount in PopoverMenuWrapper
sharanyamahajan Jan 23, 2026
5a13c99
fix: ignore whitespace-only edits in profile form initial values
sharanyamahajan Jan 24, 2026
33e5a1c
fix: stabilize public cached store cache token
sharanyamahajan Jan 25, 2026
a65a23b
refactor(livechat): replace any types in store state
sharanyamahajan Jan 26, 2026
5739d85
refactor: reduce complexity in RegisterForm
sharanyamahajan Jan 27, 2026
b037618
Merge branch 'develop' into feature/xyz
sharanyamahajan Jan 27, 2026
dcf8003
chore: standardize MONGO_OPTIONS error handling
sharanyamahajan Jan 27, 2026
55c6715
chore: finalize refactor and config updates
sharanyamahajan Jan 27, 2026
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
147 changes: 81 additions & 66 deletions apps/meteor/client/lib/cachedStores/CachedStore.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -18,18 +18,24 @@ import { getConfig } from '../utils/getConfig';

type Name = 'rooms' | 'subscriptions' | 'permissions' | 'public-settings' | 'private-settings';

const hasId = <T>(record: T): record is T & { _id: string } => typeof record === 'object' && record !== null && '_id' in record;
const hasId = <T>(record: T): record is T & { _id: string } =>
typeof record === 'object' && record !== null && '_id' in record;

const hasUpdatedAt = <T>(record: T): record is T & { _updatedAt: Date } =>
typeof record === 'object' &&
record !== null &&
'_updatedAt' in record &&
(record as unknown as { _updatedAt: unknown })._updatedAt instanceof Date;

const hasDeletedAt = <T>(record: T): record is T & { _deletedAt: Date } =>
typeof record === 'object' &&
record !== null &&
'_deletedAt' in record &&
(record as unknown as { _deletedAt: unknown })._deletedAt instanceof Date;
const hasUnserializedUpdatedAt = <T>(record: T): record is T & { _updatedAt: ConstructorParameters<typeof Date>[0] } =>

const hasUnserializedUpdatedAt = <T>(
record: T,
): record is T & { _updatedAt: ConstructorParameters<typeof Date>[0] } =>
typeof record === 'object' &&
record !== null &&
'_updatedAt' in record &&
Expand All @@ -41,31 +47,42 @@ export interface IWithManageableCache {
clearCacheOnLogout(): void;
}

export abstract class CachedStore<T extends IRocketChatRecord, U = T> implements IWithManageableCache {
export abstract class CachedStore<T extends IRocketChatRecord, U = T>
implements IWithManageableCache
{
private static readonly MAX_CACHE_TIME = 60 * 60 * 24 * 30;

readonly store: UseBoundStore<StoreApi<IDocumentMapStore<T>>>;

protected name: Name;

protected eventType: StreamNames;

private readonly version = 18;

private updatedAt = new Date(0);

protected log: (...args: any[]) => void;

private timer: ReturnType<typeof setTimeout>;

readonly useReady = create(() => false);

constructor({ name, eventType, store }: { name: Name; eventType: StreamNames; store: UseBoundStore<StoreApi<IDocumentMapStore<T>>> }) {
constructor({
name,
eventType,
store,
}: {
name: Name;
eventType: StreamNames;
store: UseBoundStore<StoreApi<IDocumentMapStore<T>>>;
}) {
this.name = name;
this.eventType = eventType;
this.store = store;

this.log = [getConfig(`debugCachedCollection-${this.name}`), getConfig('debugCachedCollection'), getConfig('debug')].includes('true')
this.log = [
getConfig(`debugCachedCollection-${this.name}`),
getConfig('debugCachedCollection'),
getConfig('debug'),
].includes('true')
? console.log.bind(console, `%cCachedCollection ${this.name}`, `color: navy; font-weight: bold;`)
: () => undefined;

Expand All @@ -82,21 +99,27 @@ export abstract class CachedStore<T extends IRocketChatRecord, U = T> implements
protected abstract getToken(): unknown;

private async loadFromCache() {
const data = await localforage.getItem<{ version: number; token: unknown; records: unknown[]; updatedAt: Date | string }>(this.name);
const data = await localforage.getItem<{
version: number;
token: unknown;
records: unknown[];
updatedAt: Date | string;
}>(this.name);

if (!data) {
return false;
}

if (data.version < this.version || data.token !== this.getToken()) {
const token = this.getToken();

if (data.version < this.version || (token !== undefined && data.token !== token)) {
return false;
}

if (data.records.length <= 0) {
return false;
}

// updatedAt may be a Date or a string depending on the used localForage backend
if (!(data.updatedAt instanceof Date)) {
data.updatedAt = new Date(data.updatedAt);
}
Expand All @@ -107,16 +130,19 @@ export abstract class CachedStore<T extends IRocketChatRecord, U = T> implements

this.log(`${data.records.length} records loaded from cache`);

const deserializedRecords = data.records.map((record) => this.deserializeFromCache(record)).filter(isTruthy);
const deserializedRecords = data.records
.map((record) => this.deserializeFromCache(record))
.filter(isTruthy);

const updatedAt = Math.max(...deserializedRecords.filter(hasUpdatedAt).map((record) => record?._updatedAt.getTime() ?? 0));
const updatedAt = Math.max(
...deserializedRecords.filter(hasUpdatedAt).map((record) => record._updatedAt.getTime()),
);

if (updatedAt > this.updatedAt.getTime()) {
this.updatedAt = new Date(updatedAt);
}

this.store.getState().replaceAll(deserializedRecords.filter(hasId));

this.updatedAt = data.updatedAt || this.updatedAt;
Comment on lines 141 to 146
Copy link
Contributor

@coderabbitai coderabbitai bot Jan 27, 2026

Choose a reason for hiding this comment

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

⚠️ Potential issue | 🟡 Minor

Potential issue with updatedAt assignment order.

Lines 141-143 compute updatedAt from deserialized records, but line 146 immediately overwrites it with data.updatedAt || this.updatedAt. This makes the computation on lines 141-143 effectively unused since it will always be overwritten.

If the intent is to use data.updatedAt as the source of truth, the computation on lines 141-143 can be removed. If the computed value should take precedence, line 146 should be removed or reordered.

🤖 Prompt for AI Agents 
In `@apps/meteor/client/lib/cachedStores/CachedStore.ts` around lines 141 - 146,
The computed updatedAt from deserializedRecords (variable updatedAt, and the
conditional that sets this.updatedAt = new Date(updatedAt)) is being overwritten
by the later assignment this.updatedAt = data.updatedAt || this.updatedAt;
either remove the earlier computation or stop overwriting it—preferably keep the
computed value as authoritative: remove the final assignment (this.updatedAt =
data.updatedAt || this.updatedAt) or change it to only assign if data.updatedAt
is strictly newer than the computed updatedAt; refer to symbols updatedAt,
deserializedRecords, hasId, this.store.getState().replaceAll, and this.updatedAt
to locate and adjust the logic.


return true;
Expand All @@ -128,28 +154,27 @@ export abstract class CachedStore<T extends IRocketChatRecord, U = T> implements
}

return {
...(record as unknown as T),
...(record as T),
...(hasUnserializedUpdatedAt(record) && {
_updatedAt: new Date(record._updatedAt),
}),
};
}

private async callLoad() {
// TODO: workaround for bad function overload
const data = await sdk.call(`${this.name}/get`);
return data as unknown as U[];
return data as U[];
}

private async callSync(updatedSince: Date) {
// TODO: workaround for bad function overload
const data = await sdk.call(`${this.name}/get`, updatedSince);
return data as unknown as { update: U[]; remove: U[] };
return data as { update: U[]; remove: U[] };
}

private async loadFromServer() {
const startTime = new Date();
const lastTime = this.updatedAt;

const data = await this.callLoad();
this.log(`${data.length} records loaded from server`);

Expand All @@ -170,16 +195,12 @@ export abstract class CachedStore<T extends IRocketChatRecord, U = T> implements
}

protected mapRecord(record: U): T {
return record as unknown as T;
return record as T;
}

protected handleLoadedFromServer(_records: T[]): void {
// This method can be overridden to handle records after they are loaded from the server
}
protected handleLoadedFromServer(_records: T[]): void {}

protected handleSyncEvent(_action: 'removed' | 'changed', _record: T): void {
// This method can be overridden to handle sync events
}
protected handleSyncEvent(_action: 'removed' | 'changed', _record: T): void {}

private async loadFromServerAndPopulate() {
await this.loadFromServer();
Expand All @@ -206,10 +227,14 @@ export abstract class CachedStore<T extends IRocketChatRecord, U = T> implements
}

protected setupListener() {
return sdk.stream(this.eventType, [this.eventName], (async (action: 'removed' | 'changed', record: U) => {
this.log('record received', action, record);
await this.handleRecordEvent(action, record);
}) as (...args: unknown[]) => void);
return sdk.stream(
this.eventType,
[this.eventName],
(async (action: 'removed' | 'changed', record: U) => {
this.log('record received', action, record);
await this.handleRecordEvent(action, record);
}) as (...args: unknown[]) => void,
);
}

protected async handleRecordEvent(action: 'removed' | 'changed', record: U) {
Expand All @@ -226,7 +251,6 @@ export abstract class CachedStore<T extends IRocketChatRecord, U = T> implements

private trySync(delay = 10) {
clearTimeout(this.timer);
// Wait for an empty queue to load data again and sync
this.timer = setTimeout(async () => {
if (!(await this.sync())) {
return this.trySync(delay);
Expand All @@ -246,14 +270,13 @@ export abstract class CachedStore<T extends IRocketChatRecord, U = T> implements
this.log(`syncing from ${this.updatedAt}`);

const data = await this.callSync(this.updatedAt);
const changes = [];
const changes: { action: () => void; timestamp: number }[] = [];

if (data.update && data.update.length > 0) {
this.log(`${data.update.length} records updated in sync`);
if (data.update?.length) {
for (const record of data.update) {
const newRecord = this.mapRecord(record);

const actionTime = hasUpdatedAt(newRecord) ? newRecord._updatedAt : startTime;

changes.push({
action: () => {
this.store.getState().store(newRecord);
Expand All @@ -267,16 +290,13 @@ export abstract class CachedStore<T extends IRocketChatRecord, U = T> implements
}
}

if (data.remove && data.remove.length > 0) {
this.log(`${data.remove.length} records removed in sync`);
if (data.remove?.length) {
for (const record of data.remove) {
const newRecord = this.mapRecord(record);

if (!hasDeletedAt(newRecord)) {
continue;
}
if (!hasDeletedAt(newRecord)) continue;

const actionTime = newRecord._deletedAt;

changes.push({
action: () => {
this.store.getState().delete(newRecord._id);
Expand All @@ -290,18 +310,31 @@ export abstract class CachedStore<T extends IRocketChatRecord, U = T> implements
}
}

changes
.sort((a, b) => a.timestamp - b.timestamp)
.forEach((c) => {
c.action();
});
changes.sort((a, b) => a.timestamp - b.timestamp).forEach((c) => c.action());

this.updatedAt = this.updatedAt === lastTime ? startTime : this.updatedAt;

return true;
}

private listenerUnsubscriber: (() => void) | undefined;
private reconnectionComputation: Tracker.Computation | undefined;
private initializationPromise: Promise<void> | undefined;

init() {
if (this.initializationPromise) {
return this.initializationPromise;
}

this.initializationPromise = this.performInitialization()
.catch(console.error)
.finally(() => {
this.initializationPromise = undefined;
this.setReady(true);
});

return this.initializationPromise;
}

private async performInitialization() {
if (await this.loadFromCache()) {
Expand All @@ -312,6 +345,7 @@ export abstract class CachedStore<T extends IRocketChatRecord, U = T> implements

this.reconnectionComputation?.stop();
let wentOffline = Tracker.nonreactive(() => Meteor.status().status === 'offline');

this.reconnectionComputation = Tracker.autorun(() => {
const { status } = Meteor.status();

Expand All @@ -331,23 +365,6 @@ export abstract class CachedStore<T extends IRocketChatRecord, U = T> implements
};
}

private initializationPromise: Promise<void> | undefined;

init() {
if (this.initializationPromise) {
return this.initializationPromise;
}

this.initializationPromise = this.performInitialization()
.catch(console.error)
.finally(() => {
this.initializationPromise = undefined;
this.setReady(true);
});

return this.initializationPromise;
}

async release() {
if (this.initializationPromise) {
await this.initializationPromise;
Expand All @@ -357,16 +374,14 @@ export abstract class CachedStore<T extends IRocketChatRecord, U = T> implements
this.setReady(false);
}

private reconnectionComputation: Tracker.Computation | undefined;

setReady(ready: boolean) {
this.useReady.setState(ready);
}
}

export class PublicCachedStore<T extends IRocketChatRecord, U = T> extends CachedStore<T, U> {
protected override getToken() {
return undefined;
return 'public';
}

override clearCacheOnLogout() {
Expand Down
19 changes: 16 additions & 3 deletions apps/meteor/client/lib/createSidebarItems.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -15,11 +15,19 @@ export type Item = {
externalUrl?: boolean;
badge?: () => ReactElement;
};
export type SidebarDivider = { divider: boolean; i18nLabel: string };

export type SidebarDivider = {
divider: boolean;
i18nLabel: string;
};

export type SidebarItem = Item | SidebarDivider;

export const isSidebarItem = (item: SidebarItem): item is Item => !('divider' in item);

export const isGoRocketChatLink = (link: string): link is `${typeof GO_ROCKET_CHAT_PREFIX}${string}` =>
export const isGoRocketChatLink = (
link: string,
): link is `${typeof GO_ROCKET_CHAT_PREFIX}${string}` =>
link.startsWith(GO_ROCKET_CHAT_PREFIX);

export const createSidebarItems = (
Expand Down Expand Up @@ -49,7 +57,12 @@ export const createSidebarItems = (

const unregisterSidebarItem = (i18nLabel: SidebarItem['i18nLabel']): void => {
const index = items.findIndex((item) => item.i18nLabel === i18nLabel);
delete items[index];

if (index === -1) {
return;
}

items.splice(index, 1);
updateCb();
};

Expand Down
Loading