Skip to content
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.

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

refactor(frontend): update multichannel route to include case Id parameter #489

Conversation

faiza-jahanzeb
Copy link
Contributor

Summary

update multichannel route to include case Id parameter

Types of changes

What types of changes does this PR introduce?
(check all that apply by placing an x in the relevant boxes)

  • 🛠️ refactoring -- non-breaking change that improves code readability or structure
  • 🐛 bugfix -- non-breaking change that fixes an issue
  • new feature -- non-breaking change that adds functionality
  • 💥 breaking change -- change that causes existing functionality to no longer work as expected
  • 📚 documentation -- changes to documentation only
  • ⚙️ build or tooling -- ex: CI/CD, dependency upgrades

Checklist

Before submitting this PR, ensure that you have completed the following. You can fill these out now, or after creating the PR.
(check all that apply by placing an x in the relevant boxes)

  • code has been linted and formatted locally
  • added or updated tests to verify the changes
  • added adequate logging for new or updated functionality
  • ensured metrics and/or tracing are in place for monitoring and debugging
  • documentation has been updated to reflect the changes (if applicable)
  • linked this PR to a related issue (if applicable)
Linting and formatting
npm run lint:check
npm run format:check
Unit and e2e tests
npm run test
npm run test:e2e

Additional Notes

If this PR introduces significant changes, explain your reasoning and provide any necessary context here. Feel free to include diagrams, screenshots, or alternative approaches you considered.

Screenshots (if applicable)

Provide screenshots or screen-recordings to help reviewers understand the visual impact of your changes, if relevant.

@faiza-jahanzeb faiza-jahanzeb enabled auto-merge (squash) March 25, 2025 18:38
Copy link
Contributor

@sebastien-comeau sebastien-comeau left a comment

Choose a reason for hiding this comment

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

Is it possible to place :caseId after /multi-channel? That would make more sense since it's a subflow within a specific case.

For example: /multi-channel/:caseId/search-sin/

@gregory-j-baker
Copy link
Contributor

Is it possible to place :caseId after /multi-channel? That would make more sense since it's a subflow within a specific case.

For example: /multi-channel/:caseId/search-sin/

If we want to follow the resource-identification convention for URLs, a better name might be /cases/:caseId/{sub-resouce} because you're pulling the case data associated with caseId.

🤷🏻

@sebastien-comeau
Copy link
Contributor

If we want to follow the resource-identification convention for URLs, a better name might be /cases/:caseId/{sub-resouce} because you're pulling the case data associated with caseId.

You're suggesting something like /cases/:caseId/multi-channel/search-sin/?

@faiza-jahanzeb faiza-jahanzeb force-pushed the faiza/update-multi-channel-routes-to-include-caseId-parameters branch from 75bf8ca to 33a5cef Compare March 26, 2025 14:07
Copy link
Contributor

@sebastien-comeau sebastien-comeau left a comment

Choose a reason for hiding this comment

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

👍

Next, find sin case using caseId params in each loaders and actions of multi-channel routes.

// TODO ::: GjB ::: the data returned by the following call should be checked to ensure the logged-in user has permissions to view it
const sinCase = await getSinCaseService().findSinCaseById(params.caseId);

if (sinCase === undefined) {
  throw new Response(JSON.stringify({ status: HttpStatusCodes.NOT_FOUND, message: 'Case not found' }), {
    status: HttpStatusCodes.NOT_FOUND,
  });
}

@faiza-jahanzeb faiza-jahanzeb merged commit 0c796fc into main Mar 26, 2025
6 checks passed
@faiza-jahanzeb faiza-jahanzeb deleted the faiza/update-multi-channel-routes-to-include-caseId-parameters branch March 26, 2025 14:17
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