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

feat: force login (close #1132) #1184

Merged
merged 35 commits into from
Jun 26, 2023
Merged

Conversation

a0m0rajab
Copy link
Contributor

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

Description

This PR closes #1132

Related Tickets & Documents

Mobile & Desktop Screenshots/Recordings

image
When the user is not logged it, it will force login.

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

@netlify
Copy link

netlify bot commented May 16, 2023

βœ… Deploy Preview for design-insights ready!

Name Link
πŸ”¨ Latest commit f7cd091
πŸ” Latest deploy log https://app.netlify.com/sites/design-insights/deploys/6499b5078495840008424b8c
😎 Deploy Preview https://deploy-preview-1184--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 settings.

@netlify
Copy link

netlify bot commented May 16, 2023

βœ… Deploy Preview for oss-insights ready!

Built without sensitive environment variables

Name Link
πŸ”¨ Latest commit f7cd091
πŸ” Latest deploy log https://app.netlify.com/sites/oss-insights/deploys/6499b507d0b465000844e32a
😎 Deploy Preview https://deploy-preview-1184--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 settings.

@brandonroberts
Copy link
Contributor

We can land this but if they select repos and are go to login, when they return the repos won't be selected

@a0m0rajab
Copy link
Contributor Author

Is there any updates I need to do for this PR?

@brandonroberts
Copy link
Contributor

@a0m0rajab does this redirect to the new insight page with the selected repos after login?

@a0m0rajab
Copy link
Contributor Author

No, I just misunderstood your first comment, wanted to know what you meant by refactoring.
I think it means something to serialize selectedRepos and send it within the URL, I will work on that.

@a0m0rajab
Copy link
Contributor Author

I think going with url?param=value approach is kinda not feasible, I tried to do that and passing the selected repos but it got me into this issue, it might require something different to do there.

The supabase redirect functioned worked one time while doing this not sure how...

image

As a solution to have shorter URL would be to pass an array with repos IDs and parse them in the app.

Beside that one thing I figured was that: the router.query condition is running multiple times in the insights page.

@brandonroberts
Copy link
Contributor

@a0m0rajab yes, only passing selectRepos as just the IDs would be a better fix. Then you'd have to fetch the repos using the selectedRepos in the InsightPage.tsx component and add them to the listed repos.

@a0m0rajab
Copy link
Contributor Author

Thank you for that, Just another solution I got in mind is using localStorage to get the repos as well.
If the query params did not work, I might work on this one.

@brandonroberts
Copy link
Contributor

brandonroberts commented May 30, 2023

We discussed using local storage previously and its too prone to error with unpredictable navigation

@OgDev-01
Copy link
Contributor

OgDev-01 commented Jun 2, 2023

Hey @a0m0rajab, have you been able to try @brandonroberts suggestion?... Let me know if you are blocked anytime πŸ‘

@a0m0rajab
Copy link
Contributor Author

@OgDev-01 Thank you for asking, I just did not find time recently, might do it at the weekend.
@brandonroberts Thank you for letting me know, I will go with passing IDs and see how things will work.

@a0m0rajab
Copy link
Contributor Author

a0m0rajab commented Jun 5, 2023

Got blocked while working on this, had the same long URI issue from supabase this time.
I feel either going with localstorage would solve this, what errors have you faced while doing such approach? (I used local storage previously without getting any issues)

@takanome-dev
Copy link
Contributor

Hey @a0m0rajab, the Supabase redirect issue was fixed in #1240. Try pulling the latest changes and try it again. Let us know if you're still facing an issue πŸ•

@a0m0rajab
Copy link
Contributor Author

@takanome-dev it did not work, still the same issue,

image

I think it was my mistake, just solved the issue, had to remove a test code.

@a0m0rajab
Copy link
Contributor Author

@brandonroberts is this what you are talking about?
I think it's an expected behavior:

2023-06-22.00-18-04.mp4

Or do you mean that if they logged out in the add new insight page, we need to stay at the same page?

@brandonroberts
Copy link
Contributor

@a0m0rajab I was logged out, I selected some repos, and clicked "Add to Insight Page". It took me to login, and brought me back but didn't keep me on the "Create Insight Page" with the selected repos

@a0m0rajab
Copy link
Contributor Author

@brandonroberts could it be a cache issue? I tried this with incognito mode and it worked.

2023-06-22.17-16-05.mp4

@brandonroberts
Copy link
Contributor

@a0m0rajab I haven't tried it locally, but only through the PR deploy preview. I tried it with incognito mode also

@a0m0rajab
Copy link
Contributor Author

I will add a few debuggers to test the issue.

@a0m0rajab
Copy link
Contributor Author

@brandonroberts i tested it on the preview and it worked, I had to change the remove login to signout.

@a0m0rajab
Copy link
Contributor Author

Latest video:

2023-06-23.11-23-58.mp4

Copy link
Contributor

@brandonroberts brandonroberts left a comment

Choose a reason for hiding this comment

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

Minor tweak

layouts/private-wrapper.tsx Outdated Show resolved Hide resolved
@brandonroberts
Copy link
Contributor

@a0m0rajab everything works correctly now. Remove the console.log statements and we're good to go

@a0m0rajab
Copy link
Contributor Author

Done πŸ‘

@brandonroberts brandonroberts merged commit 21aaa0b into open-sauced:beta Jun 26, 2023
10 checks passed
github-actions bot pushed a commit that referenced this pull request Jun 26, 2023
## [1.54.0-beta.4](v1.54.0-beta.3...v1.54.0-beta.4) (2023-06-26)

### πŸ• Features

* allow login flow after selecting repositories to add to insight page (close [#1132](#1132)) ([#1184](#1184)) ([21aaa0b](21aaa0b))
@github-actions
Copy link

πŸŽ‰ This PR is included in version 1.54.0-beta.4 πŸŽ‰

The release is available on GitHub release

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

github-actions bot pushed a commit that referenced this pull request Jun 28, 2023
## [1.54.0](v1.53.0...v1.54.0) (2023-06-28)

### πŸ› Bug Fixes

* fix yellow color for `<Pill />` component ([#1299](#1299)) ([2f2d9f5](2f2d9f5))

### πŸ• Features

*  hide highlights tab components on profiles without highlights ([#1304](#1304)) ([976739f](976739f))
* add `BillBoard` component to design system ([#1306](#1306)) ([1181cfd](1181cfd))
* add `FeaturedHighlightPanel` component to design system ([#1307](#1307)) ([452213c](452213c))
* add `UserCard` component to storybook design system ([#1295](#1295)) ([6867011](6867011))
* add highlight prompt to design system ([#1297](#1297)) ([4a85e74](4a85e74))
* add top users panel component to design system ([#1300](#1300)) ([9c05fec](9c05fec))
* add URL for collaboration requests ([#1305](#1305)) ([b3f8ea5](b3f8ea5))
* allow login flow after selecting repositories to add to insight page (close [#1132](#1132)) ([#1184](#1184)) ([21aaa0b](21aaa0b))
* implemented/added scroll-area component to design-system  ([#1283](#1283)) ([b7280ab](b7280ab))
@github-actions
Copy link

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

The release is available on GitHub release

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

@a0m0rajab a0m0rajab deleted the force-login branch July 30, 2023 23:31
ElpisHelle added a commit to ElpisHelle/next.js-tailwindcss that referenced this pull request Aug 17, 2023
## [1.54.0-beta.4](open-sauced/app@v1.54.0-beta.3...v1.54.0-beta.4) (2023-06-26)

### πŸ• Features

* allow login flow after selecting repositories to add to insight page (close [#1132](open-sauced/app#1132)) ([#1184](open-sauced/app#1184)) ([21aaa0b](open-sauced/app@21aaa0b))
ElpisHelle added a commit to ElpisHelle/next.js-tailwindcss that referenced this pull request Aug 17, 2023
## [1.54.0](open-sauced/app@v1.53.0...v1.54.0) (2023-06-28)

### πŸ› Bug Fixes

* fix yellow color for `<Pill />` component ([#1299](open-sauced/app#1299)) ([2f2d9f5](open-sauced/app@2f2d9f5))

### πŸ• Features

*  hide highlights tab components on profiles without highlights ([#1304](open-sauced/app#1304)) ([976739f](open-sauced/app@976739f))
* add `BillBoard` component to design system ([#1306](open-sauced/app#1306)) ([1181cfd](open-sauced/app@1181cfd))
* add `FeaturedHighlightPanel` component to design system ([#1307](open-sauced/app#1307)) ([452213c](open-sauced/app@452213c))
* add `UserCard` component to storybook design system ([#1295](open-sauced/app#1295)) ([6867011](open-sauced/app@6867011))
* add highlight prompt to design system ([#1297](open-sauced/app#1297)) ([4a85e74](open-sauced/app@4a85e74))
* add top users panel component to design system ([#1300](open-sauced/app#1300)) ([9c05fec](open-sauced/app@9c05fec))
* add URL for collaboration requests ([#1305](open-sauced/app#1305)) ([b3f8ea5](open-sauced/app@b3f8ea5))
* allow login flow after selecting repositories to add to insight page (close [#1132](open-sauced/app#1132)) ([#1184](open-sauced/app#1184)) ([21aaa0b](open-sauced/app@21aaa0b))
* implemented/added scroll-area component to design-system  ([#1283](open-sauced/app#1283)) ([b7280ab](open-sauced/app@b7280ab))
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.

Feature: Force login when checking the box
5 participants