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

1862 - refine multi zone routing #1884

Merged
merged 18 commits into from
Jul 3, 2024

Conversation

marcellmueller
Copy link
Contributor

@marcellmueller marcellmueller commented Jun 26, 2024

PR to bring in some updates from #1862 that weren't brought in with #1867

View #1867 for some more context on this though what I've brought in this PR:

  • Updates withAuthorization middlewares
  • Adds tests for these middlewares
  • Added dev dependency ts-mockito to help mock NextResponse. It's a cool library that generates mocks out of types, could come in handy in the future
  • Move action handler to actions project, update imports

@marcellmueller marcellmueller changed the title chore: update with authorization 1862 - refine multi zone routing Jun 26, 2024
@marcellmueller marcellmueller force-pushed the 1862-refine-multi-zone-routing branch 2 times, most recently from 9c5318e to 075e862 Compare June 27, 2024 20:45
import { NextURL } from "next/dist/server/web/next-url";
import { NextFetchEvent, NextRequest, NextResponse } from "next/server";
import { instance, mock, reset, when } from "ts-mockito";
import middleware from "../middleware";
Copy link
Contributor Author

@marcellmueller marcellmueller Jun 28, 2024

Choose a reason for hiding this comment

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

Technically this is testing bciers/apps/dashboard/middleware.ts though this currently is the only middleware and all the tests are about this so I thought this made more sense. Also I wasn't sure how to just test this withAuthorization function on its own but found examples online of how to test the NextJS middleware stack.

@marcellmueller marcellmueller marked this pull request as ready for review June 28, 2024 15:35
@marcellmueller marcellmueller force-pushed the 1862-refine-multi-zone-routing branch 3 times, most recently from 93b9367 to 601e587 Compare July 2, 2024 14:41
email?: string;
phone_number?: string;
app_role?: { role_name: string };
}
Copy link
Contributor

Choose a reason for hiding this comment

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

I don't see why we need the UserProfileFormData and UserProfilePartialFormData interfaces defined again here when they're already defined in bciers/libs/shared/types/src/form/formData.ts ?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Thanks, fixed!

@marcellmueller marcellmueller force-pushed the 1862-refine-multi-zone-routing branch 2 times, most recently from 77f04da to e91fd90 Compare July 3, 2024 18:40
@andrea-williams andrea-williams self-requested a review July 3, 2024 21:54
Copy link
Contributor

@andrea-williams andrea-williams left a comment

Choose a reason for hiding this comment

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

Can confirm that everything works peachy when I actually set up my local env properly... 🙃

@marcellmueller marcellmueller force-pushed the 1862-refine-multi-zone-routing branch 2 times, most recently from 155554b to 9a45e39 Compare July 3, 2024 22:37
@marcellmueller marcellmueller merged commit cfa345f into develop Jul 3, 2024
45 checks passed
@marcellmueller marcellmueller deleted the 1862-refine-multi-zone-routing branch July 3, 2024 22:55
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.

2 participants