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: add guest nickname handling #33

Closed
wants to merge 1 commit into from
Closed

Conversation

skjnldsv
Copy link
Contributor

@skjnldsv skjnldsv added 3. to review Waiting for reviews enhancement New feature or request labels Jul 17, 2024
@skjnldsv skjnldsv self-assigned this Jul 17, 2024
lib/publicShare.ts Outdated Show resolved Hide resolved
@skjnldsv skjnldsv requested a review from ShGKme July 17, 2024 12:44
@nickvergessen nickvergessen requested review from Antreesy and removed request for nickvergessen July 17, 2024 12:44
@skjnldsv skjnldsv force-pushed the feat/shared-nick branch 2 times, most recently from 0a82768 to d3213f8 Compare July 17, 2024 12:48
Copy link

@ShGKme ShGKme left a comment

Choose a reason for hiding this comment

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

Makes sense to fix both issues. But seems unrelated to sharing library, or do I get meaning of this library wrong?

lib/publicShare.ts Outdated Show resolved Hide resolved
@skjnldsv
Copy link
Contributor Author

But seems unrelated to sharing library, or do I get meaning of this library wrong?

Tough discussion. I think it fits tbh. We already have some public sharing handling here.
Every app that uses the Guest nickname would be in a context of sharing (talk conversation share link, file/folder share link, poll.... etc

So I think this is where it makes more sense :)
Not to be confused with the guest app here ⚠️

Signed-off-by: skjnldsv <skjnldsv@protonmail.com>
@susnux
Copy link
Contributor

susnux commented Jul 17, 2024

Hm I am not that sure, as this library is more about files sharing (potentially we should add the Share model etc to this library).

For me this part should probably be in the same library as getCurrentUser so auth?

@susnux
Copy link
Contributor

susnux commented Jul 17, 2024

(code looks fine tough)

@skjnldsv
Copy link
Contributor Author

potentially we should add the Share model etc to this library

Our entire sharing backend allows any resources to be shared, not only file :)
Theoretically. So any sharing related thing wuold belong here I would say 🤔

Nonetheless, I would also agree, I like the idea to have this on auth.
Since we're a few that will use that one, I'll let you all decide 👀

@juliushaertl
Copy link
Contributor

Auth sounds even more fitting to me

@skjnldsv
Copy link
Contributor Author

Let's gooo! The people have spoken! 🧑‍⚖️

@skjnldsv
Copy link
Contributor Author

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
3. to review Waiting for reviews enhancement New feature or request
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Unified/shared storage + fields e.g. for guest names Global guest name setting
4 participants