Skip to content

feat(documents): add document table and queries#12

Open
AmritBhusal wants to merge 4 commits into
developfrom
documents
Open

feat(documents): add document table and queries#12
AmritBhusal wants to merge 4 commits into
developfrom
documents

Conversation

@AmritBhusal

@AmritBhusal AmritBhusal commented Jul 1, 2026

Copy link
Copy Markdown
Contributor

Changes

  • add Documents tables and table actions
  • add table filters and search
  • add new route

This PR doesn't introduce any

  • temporary files, auto-generated files or secret keys
  • build works
  • eslint issues
  • typescript issues
  • codegen errors
  • console.log meant for debugging
  • typos
  • unwanted comments
  • conflict markers

@AmritBhusal AmritBhusal marked this pull request as ready for review July 1, 2026 10:12
@AmritBhusal AmritBhusal requested review from crsstha, shreeyash07 and subinasr and removed request for crsstha, shreeyash07 and subinasr July 1, 2026 10:16
@AmritBhusal AmritBhusal marked this pull request as draft July 9, 2026 05:18
@AmritBhusal AmritBhusal marked this pull request as ready for review July 9, 2026 05:37
contentType: ReportContentType.File,
};

const MAX_FILE_SIZE = 5 * 1024 * 1024; // 5MB

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

lets make this common constant


const handleFileChange = useCallback((file: File | undefined) => {
if (isDefined(file) && file.size > MAX_FILE_SIZE) {
alert.show('File must be 5MB or smaller', { variant: 'danger' });

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

we need to show this in non field error instead of alert, i think FileInput have size props to handle this.

@subinasr subinasr left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

From now onwards, please try to adhere to the branch naming conventions.
For reference: https://notes.tools.togglecorp.com/6MN_ghZfS02ucm__t6_WCQ#

} from '@ifrc-go/ui';
import { type EntriesAsList } from '@togglecorp/toggle-form';

import type { DocumentFilterType } from '../index';

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Suggested change
import type { DocumentFilterType } from '../index';
import type { DocumentFilterType } from '..';

function isAllowedReportType(value: string | null | undefined): value is ReportTypeEnum {
return isDefined(value) && (allowedReportTypes as string[]).includes(value);
}

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Way too many type casts. There might be a way to simpler write the same function.

} = useForm(documentSchema, { value: initialValue });

const [{ data, fetching: documentDetailFetch }] = useDocumentDetailQuery({
variables: { id: isDefined(id) ? id : '' },

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Why not send undefined when there's no id?

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

undefined causes a type error (id is generated as string), so ''

Comment on lines +138 to +142
const reportTypeOptions = useMemo(() => (
(enumsData?.enums?.ReportType ?? []).filter(
(option) => allowedReportTypes.includes(option.key),
)
), [enumsData]);

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

I believe there's useGlobalEnums hook you can use instead of separately fetching the enums.

Comment on lines +226 to +237
useEffect(() => {
if (!documentDetailFetch && isDefined(documentData)) {
const {
...otherValues
} = removeNull(documentData);

delete (otherValues as { file?: unknown }).file;
setValue({
...otherValues,
});
}
}, [documentDetailFetch, documentData, setValue]);

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Can you check this properly? This can be done without so much destructuring and mutation of the object.

title="File Upload"
description="Upload the document file"
withAsteriskOnTitle

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Remove this line

Comment thread app/views/Documents/index.tsx Outdated
Comment on lines +67 to +73
function persistTab(value: string | null | undefined): ReportTypeEnum {
if (value === ReportTypeEnum.Policy || value === ReportTypeEnum.Guideline) {
return value;
}
return ReportTypeEnum.Manual;
}

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

This function is unnecessary.

Comment thread app/views/Documents/index.tsx Outdated
const tableData: ReportsListItem[] = useMemo(() => (
(data?.reports?.results ?? []).map((report, index) => ({
...report,
no: String((page - 1) * limit + index + 1),

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

You can keep this as a number unless you have a specific reason for the type cast.

Comment thread app/utils/common.ts Outdated

import type { AdminAreaLevel } from '#generated/types/graphql';

export const MAX_FILE_SIZE = 5 * 1024 * 1024; // 5MB

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Let's use two common variables here. MAX_REPORT_FILE_SIZE and MAX_IMAGE_FILE_SIZE as the limits for the two are different.

Comment thread app/components/FileInput/index.tsx Outdated
Comment on lines +44 to +45
setSizeError(`File must be ${Math.round(maxSize / (1024 * 1024))}MB or smaller`);
onChange(undefined, inputName);

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Is this necessary? What do you think @shreeyash07 ?

@AmritBhusal AmritBhusal requested a review from subinasr July 10, 2026 04:10
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.

4 participants