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.

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

Jump to bottom

01 09 chore reuse get server side props #170

Open
wants to merge 12 commits into
base: main
Choose a base branch
from
Open

01 09 chore reuse get server side props #170

wants to merge 12 commits into from

Conversation

DmytroHryshyn
Copy link

What does this PR do?

Fixes # (issue)

Requirement/Documentation

  • If there is a requirement document, please, share it here.
  • If there is ab UI/UX design document, please, share it here.

Type of change

  • Bug fix (non-breaking change which fixes an issue)
  • Chore (refactoring code, technical debt, workflow improvements)
  • New feature (non-breaking change which adds functionality)
  • Breaking change (fix or feature that would cause existing functionality to not work as expected)
  • Tests (Unit/Integration/E2E or any other test)
  • This change requires a documentation update

How should this be tested?

  • Are there environment variables that should be set?
  • What are the minimal test data to have?
  • What is expected (happy path) to have (input and output)?
  • Any other important info that could help to test that PR

Mandatory Tasks

  • Make sure you have self-reviewed the code. A decent size PR without self-review might be rejected.

Checklist

  • I haven't read the contributing guide
  • My code doesn't follow the style guidelines of this project
  • I haven't commented my code, particularly in hard-to-understand areas
  • I haven't checked if my PR needs changes to the documentation
  • I haven't checked if my changes generate no new warnings
  • I haven't added tests that prove my fix is effective or that my feature works
  • I haven't checked if new and existing unit tests pass locally with my changes

@github-actions GitHub Actions
Copy link

github-actions bot commented Jan 9, 2024 •
edited
Loading

Hey there and thank you for opening this pull request! 👋🏼

We require pull request titles to follow the Conventional Commits specification and it looks like your proposed title needs to be adjusted.

Details:

No release type found in pull request title "01 09 chore reuse get server side props". Add a prefix to indicate what kind of release this pull request corresponds to. For reference, see https://www.conventionalcommits.org/

Available types:
 - feat: A new feature
 - fix: A bug fix
 - docs: Documentation only changes
 - style: Changes that do not affect the meaning of the code (white-space, formatting, missing semi-colons, etc)
 - refactor: A code change that neither fixes a bug nor adds a feature
 - perf: A code change that improves performance
 - test: Adding missing tests or correcting existing tests
 - build: Changes that affect the build system or external dependencies (example scopes: gulp, broccoli, npm)
 - ci: Changes to our CI configuration files and scripts (example scopes: Travis, Circle, BrowserStack, SauceLabs)
 - chore: Other changes that don't modify src or test files
 - revert: Reverts a previous commit

grzpab
grzpab reviewed Jan 11, 2024
View reviewed changes
apps/web/app/future/apps/installed/[category]/page.tsx Outdated
return <LegacyPage />;
}
// @ts-expect-error Argument of type '(ctx: AppGetServerSidePropsContext) => Promise<{ redirect: { destination: string; permanent: boolean; }; notFound?: undefined; props?: undefined; }
export default WithLayout({ getLayout: null, getData: withAppDirSsr(getServerSideProps), Page });
Copy link

Choose a reason for hiding this comment

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

can we remove the @ts-expect-error directive from here?

apps/web/app/future/video/no-meeting-found/page.tsx Outdated
@@ -12,8 +12,8 @@ export const generateMetadata = async () =>
(t) => t("no_meeting_found")
);

// ssr was added by Intuita, legacy page does not have it
const getData = async (context: ReturnType<typeof buildLegacyCtx>) => {
Copy link

Choose a reason for hiding this comment

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

shouldn't we change the type of context here to GetServerSidePropsContext?

packages/app-store/alby/pages/setup/_getServerSideProps.tsx Outdated
if (!session?.user?.id) {
return res.writeHead(401).end();
}
// if (!session?.user?.id) {
Copy link

Choose a reason for hiding this comment

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

I think this should be restored?

@DmytroHryshyn DmytroHryshyn force-pushed the 01-09-chore_reuse_getServerSideProps branch 2 times, most recently from c5f0aaa to 8393c8c Compare January 11, 2024 16:16
@grzpab grzpab force-pushed the 01-09-chore_reuse_getServerSideProps branch from 03e41ac to 04681e8 Compare January 12, 2024 09:05
@DmytroHryshyn DmytroHryshyn force-pushed the 01-09-chore_reuse_getServerSideProps branch 5 times, most recently from 0ced69a to 37f46b9 Compare January 15, 2024 09:24
@hbjORbj hbjORbj force-pushed the 01-09-chore_reuse_getServerSideProps branch 3 times, most recently from ee14b31 to e3d3377 Compare January 16, 2024 17:53
@hbjORbj hbjORbj force-pushed the 01-09-chore_reuse_getServerSideProps branch from 1551022 to a699489 Compare January 17, 2024 15:04
@github-actions GitHub Actions
Copy link

github-actions bot commented Feb 3, 2024

This PR is being marked as stale due to inactivity.

@github-actions github-actions bot added the Stale label Feb 3, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@grzpab grzpab grzpab approved these changes

Assignees
No one assigned
Labels
❗️ migrations Stale
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
4 participants
@DmytroHryshyn @grzpab @hbjORbj @zomars