Skip to content

added ResourceDetail + ConfirmModal (DEV-78)#22

Open
nuthanan06 wants to merge 2 commits intomainfrom
feature/DEV-78/resource-detail
Open

added ResourceDetail + ConfirmModal (DEV-78)#22
nuthanan06 wants to merge 2 commits intomainfrom
feature/DEV-78/resource-detail

Conversation

@nuthanan06
Copy link
Contributor

@nuthanan06 nuthanan06 commented Mar 6, 2026

Jira ticket link

Jira ticket

Implementation description

  • Added a generic + dynamic ResourceDetail component that lists out fields of any object
  • Also added a confirm modal that allows users to delete a certain object of their chosing

Steps to test

  1. Navigate to http://localhost:8000/docs and add agencies via POST request
  2. Navigate to http://localhost:3000/agencies and verify that all agencies are added are see.
  3. Click on each agency to view the specific details, verify that all fields in the POST request are reflected in the ResoruceDetail
  4. Test a delete, on the top right, press confirm delete to open up the ConfirmModal, and then verify that the agency that was deleted is no longer in the ResourceDetail

What should reviewers focus on?

  • The generic ResourceDetail component works for viewing any resource.
  • The component is tested with the Agencies API to fetch and display agency details.
  • The delete action is integrated into the ResourceDetail and works with the Agencies API (DELETE /api/agencies/{id}).
  • A ConfirmModal is shown for the delete action, and upon confirmation, the agency is deleted.

Checklist

Format for branch, commit, and PR title: docs/GIT.md.

  • My branch name includes the Jira ticket key
  • My PR name is descriptive and in imperative tense
  • My PR name includes the Jira ticket key
  • My commit messages are descriptive and in imperative tense. My commits are atomic and trivial commits are squashed or fixup'd into non-trivial commits
  • My commit messages include the Jira ticket key
  • I have run the appropriate linter(s)
  • I have requested a review from the PL, as well as other devs who have background knowledge on this PR or who will be building on top of this PR

Copilot AI review requested due to automatic review settings March 6, 2026 01:41
Copy link

Copilot AI left a comment

Choose a reason for hiding this comment

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

Pull request overview

This PR adds a reusable “resource detail” UI pattern (detail view + delete confirmation) and wires it into the Agencies feature by introducing an agency detail route with fetch + delete hooks.

Changes:

  • Add generic ResourceDetail + ConfirmModal components and shared types to support detail rendering and delete confirmation.
  • Introduce React Query hooks to fetch a single agency and delete an agency with cache invalidation.
  • Add /agencies/[id] detail page and link to it from the agencies list.

Reviewed changes

Copilot reviewed 6 out of 7 changed files in this pull request and generated 6 comments.

Show a summary per file
File Description
frontend/types/index.ts Adds shared generic prop/type definitions for ResourceDetail.
frontend/tsconfig.tsbuildinfo Adds a TypeScript build artifact (should not be committed).
frontend/hooks/useApi.ts Adds useAgency (fetch by id) and useDeleteAgency (delete + invalidate caches).
frontend/components/ResourceDetail.tsx New reusable detail renderer with delete action that triggers ConfirmModal.
frontend/components/ConfirmModal.tsx New confirmation dialog component used for delete flows.
frontend/app/agencies/page.tsx Adds “View details” link for each agency to navigate to the new detail route.
frontend/app/agencies/[id]/page.tsx New agency detail page using ResourceDetail, fetch hook, and delete hook.

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

@nuthanan06 nuthanan06 marked this pull request as ready for review March 6, 2026 02:05
@nuthanan06 nuthanan06 requested a review from kenzysoror March 6, 2026 02:05
@kenzysoror kenzysoror requested a review from ivymxu March 6, 2026 13:22
key: "billing_profiles",
label: "Billing Profiles",
emptyValue: "None",
},
Copy link

Choose a reason for hiding this comment

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

defaultDisplayValue will call JSON.stringify on the profiles array, producing raw output so maybe add a render like this: (v) => Array.isArray(v) ? ${v.length} profile(s) : "-"

const params = useParams<{ id: string }>();
const router = useRouter();
const rawId = params?.id;
const agencyId = Array.isArray(rawId) ? rawId[0] : rawId;
Copy link

Choose a reason for hiding this comment

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

I think Array.isArray branch is dead code so we can simplify it to:

Suggested change
const agencyId = Array.isArray(rawId) ? rawId[0] : rawId;
const agencyId = params?.id

}: ResourceDetailProps<T>) {
const [showConfirmModal, setShowConfirmModal] = useState(false);

const handleDeleteConfirm = async () => {
Copy link

Choose a reason for hiding this comment

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

If onDelete() rejects, setShowConfirmModal(false) is never reached so should there be error handling here?

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.

3 participants