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: merge authSession function into useSession hook #1391

Conversation

babblebey
Copy link
Contributor

@babblebey babblebey commented Jul 19, 2023

Description

This pull request refactors the useSession hook and incorporates (and deprecates) the functionality of the deprecated authSession function, resulting in an improved and more organized approach to handle session-related data and authentication.

Changes Made

  1. Merged the logic from the deprecated authSession function into the useSession hook to consolidate related functionalities and enhance code maintainability.
  2. Replaced the use of the authSession function with the new loadSession function, which performs the same session data retrieval from the backend API, and made the function available as a returned value under the same authSession name making it accessible to users if needed.
  3. Separated the state management for session data and the store update logic into two distinct functions: loadSession and setStoreData.
  4. The loadSession function now handles the API call to fetch session data and returns the parsed data when the response is successful. In case of an error, it returns false.
  5. The setStoreData function is responsible for updating the store state with the obtained session data, keeping the code more modular and readable.
  6. Added deprecation notice inside the authSession function stating its migration to the useSession hook.
  7. Updated the usage of the authSession with the useSession in the codebase.

What type of PR is this? (check all applicable)

  • πŸ• Feature
  • πŸ› Bug Fix
  • πŸ“ Documentation Update
  • 🎨 Style
  • πŸ§‘β€πŸ’» Code Refactor
  • πŸ”₯ Performance Improvements
  • βœ… Test
  • πŸ€– Build
  • πŸ” CI
  • πŸ“¦ Chore (Release)
  • ⏩ Revert

Related Tickets & Documents

Fixes #1169

Mobile & Desktop Screenshots/Recordings

Added tests?

  • πŸ‘ yes
  • πŸ™… no, because they aren't needed
  • πŸ™‹ no, because I need help

Added to documentation?

  • πŸ“œ README.md
  • πŸ““ docs.opensauced.pizza
  • πŸ• dev.to/opensauced
  • πŸ“• storybook
  • πŸ™… no documentation needed

[optional] Are there any post-deployment tasks we need to perform?

[optional] What gif best describes this PR or how it makes you feel?

@netlify
Copy link

netlify bot commented Jul 19, 2023

βœ… Deploy Preview for design-insights ready!

Name Link
πŸ”¨ Latest commit 31add71
πŸ” Latest deploy log https://app.netlify.com/sites/design-insights/deploys/64c02e996e53000008427c5f
😎 Deploy Preview https://deploy-preview-1391--design-insights.netlify.app
πŸ“± Preview on mobile
Toggle QR Code...

QR Code

Use your smartphone camera to open QR code link.

To edit notification comments on pull requests, go to your Netlify site configuration.

@netlify
Copy link

netlify bot commented Jul 19, 2023

βœ… Deploy Preview for oss-insights ready!

Built without sensitive environment variables

Name Link
πŸ”¨ Latest commit 31add71
πŸ” Latest deploy log https://app.netlify.com/sites/oss-insights/deploys/64c02e99704709000857f4d7
😎 Deploy Preview https://deploy-preview-1391--oss-insights.netlify.app
πŸ“± Preview on mobile
Toggle QR Code...

QR Code

Use your smartphone camera to open QR code link.

To edit notification comments on pull requests, go to your Netlify site configuration.

@babblebey babblebey changed the title refactor!: merge auth session into use session hook refactor!: merge authSession function into useSession hook Jul 19, 2023
@babblebey babblebey marked this pull request as ready for review July 20, 2023 13:37
lib/hooks/authSession.ts Outdated Show resolved Hide resolved
@brandonroberts
Copy link
Contributor

@babblebey maybe I miscommunicated the intent here. There should only be the useSession hook. authSession should not be exported publicly. The hook should make the request where needed to initiate the session, and return the properties from the store to be consumed.

@babblebey
Copy link
Contributor Author

babblebey commented Jul 21, 2023

Done @brandonroberts

I had few ideas to approach this in mind but here's what I settled for...

Return a local session state value from the useSession hook in place of the authSession

This approach means that we'll keep/manage a local state session within the useSession hook, this session state value will be set on receipt of response from the /auth/session endpoint and then it is returned alongside the store managed properties (i.e. onboarded, waitlisted & hasReports)

const useSession = (getSession = false) => {
  //...
  const [session, setSession] = useState(); // <- Initialising State πŸ•

  useEffect(() => {
    (async () => {
      if (sessionToken && getSession) {
        const data = await loadSession();
        setSession(data); // <- Setting the State πŸ•
        setStoreData(...);
      }
    })();
  }, [sessionToken]);

  return { 
    onboarded, 
    waitlisted, 
    hasReports, 
    session // <- Returned State πŸ•
  };
};

With this approach, It means we

  • can re-fetch session from the /auth/session endpoint and destructure its response as session state value every time we call useSession with the getSession argument set to true without an Initial value retrieved from anywhere or newly set value saved anywhere outside the useSession hook.
const { session } = useSession(true)
  • retain the the ability to really not get any session data when getSession is kept at default false, and we are still able to retrieve the store saved properties (i.e. onboarded, waitlisted & hasReports), these values are already initialised in the _app.tsx component.

  • simply use the destructured session state value from useSession(true) in any component we wish to integrate it.
    Usage in AuthSection component

// components\molecules\AuthSection\auth-section.tsx

const { onboarded, session } = useSession(true);

useEffect(() => {
  const getUser = async () => {
    if (session && !userInfo) {
      setUserInfo(session);
    }
  };

  getUser();
}, [userInfo]);

Like so πŸ•

@brandonroberts brandonroberts added merge conflicts Needs merge conflict resolved and removed requested changes labels Jul 25, 2023
@brandonroberts
Copy link
Contributor

@babblebey changes look good thanks. After the merge conflicts are resolved, its good to go

@babblebey
Copy link
Contributor Author

Phew πŸ•

@OgDev-01 OgDev-01 removed the merge conflicts Needs merge conflict resolved label Jul 25, 2023
@OgDev-01 OgDev-01 merged commit d6c230d into open-sauced:beta Jul 25, 2023
11 checks passed
github-actions bot pushed a commit that referenced this pull request Jul 25, 2023
### [1.57.1-beta.6](v1.57.1-beta.5...v1.57.1-beta.6) (2023-07-25)

### πŸ§‘β€πŸ’» Code Refactoring

*  merge `authSession` function into `useSession` hook ([#1391](#1391)) ([d6c230d](d6c230d))
@github-actions
Copy link

πŸŽ‰ This PR is included in version 1.57.1-beta.6 πŸŽ‰

The release is available on GitHub release

Your semantic-release bot πŸ“¦πŸš€

github-actions bot pushed a commit that referenced this pull request Aug 2, 2023
## [1.58.0](v1.57.0...v1.58.0) (2023-08-02)

### πŸ§‘β€πŸ’» Code Refactoring

*  merge `authSession` function into `useSession` hook ([#1391](#1391)) ([d6c230d](d6c230d))

### 🎨 Styles

* add width and max width for created at in contributor-profile-tab ([#1429](#1429)) ([5614886](5614886))
* remove irrelevant padding in page divider ([#1427](#1427)) ([5faaa4e](5faaa4e))

### πŸ• Features

* add devcard button to user profile ([#1339](#1339)) ([6a1dbdc](6a1dbdc))
* new contributor highlight card ([#1443](#1443)) ([c88000b](c88000b))
* update redirect to feed page for unauthenticated users ([#1464](#1464)) ([6d8505f](6d8505f))

### πŸ› Bug Fixes

* add checkbox id name if not available based on label ([#1466](#1466)) ([68f66a7](68f66a7))
* add navigation to improve accessibility ([#1436](#1436)) ([d1d85f7](d1d85f7))
* contributor profile tab click state flicker ([#1432](#1432)) ([c9cf8ed](c9cf8ed))
* Deleted page button changed to delete highlight ([#1419](#1419)) ([d502605](d502605))
* fixed bug on chatbot button overlay ([#1420](#1420)) ([2a94583](2a94583))
* hide onboarding button on mobile ([#1460](#1460)) ([f63f240](f63f240))
* Improve Keyboard Accessibility for Notification Icon ([#1435](#1435)) ([05291c0](05291c0))
* improve layout design for large screens [#1231](#1231) ([#1437](#1437)) ([d8ae808](d8ae808))
* misaligned chat button close icon ([#1422](#1422)) ([60f22bd](60f22bd))
* mismatched selection color ([#1430](#1430)) ([8a1d37b](8a1d37b))
* reduce tab font size in contributors profile page ([#1413](#1413)) ([238dc2f](238dc2f))
* show repo filters on initial `feeds` route visit ([#1426](#1426)) ([833ee30](833ee30))
* tab inconsistency in user profile matching the url ([#1403](#1403)) ([f8c6766](f8c6766))
@github-actions
Copy link

github-actions bot commented Aug 2, 2023

πŸŽ‰ This PR is included in version 1.58.0 πŸŽ‰

The release is available on GitHub release

Your semantic-release bot πŸ“¦πŸš€

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

refactor: merge the useSession and authSession hooks into one hook
3 participants