Skip to content

share with teams #1465

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
Merged
juliusknorr merged 1 commit into from
Jan 9, 2025
Merged

share with teams #1465

juliusknorr merged 1 commit into from
Jan 9, 2025

Conversation

@hweihwang
Copy link
Contributor

@hweihwang hweihwang commented Nov 18, 2024 •
edited by juliusknorr
Loading

Fixes #142

@hweihwang hweihwang force-pushed the share-with-teams branch 5 times, most recently from 8a288e5 to a6da886 Compare November 18, 2024 10:12
@juliusknorr juliusknorr requested review from blizzz and enjeck November 18, 2024 13:10
@juliusknorr juliusknorr added enhancement New feature or request 2. developing Work in progress labels Nov 18, 2024
@hweihwang hweihwang linked an issue Nov 18, 2024 that may be closed by this pull request
Share with Teams #142
Closed
blizzz
blizzz requested changes Nov 18, 2024
View reviewed changes
Copy link
Member

@blizzz blizzz left a comment

Choose a reason for hiding this comment

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

I looked at the backend bits only so far.

@hweihwang
Copy link
Contributor Author

hweihwang commented Nov 19, 2024

Thanks Arthur @blizzz 🙏 - those are really helpful!

@hweihwang hweihwang force-pushed the share-with-teams branch 2 times, most recently from 93927cb to a36c56e Compare November 20, 2024 10:21
@hweihwang hweihwang marked this pull request as ready for review November 20, 2024 10:22
@hweihwang hweihwang force-pushed the share-with-teams branch 6 times, most recently from 7803db6 to 34d53d6 Compare November 22, 2024 07:05
@hweihwang hweihwang requested a review from blizzz November 22, 2024 07:27
enjeck

This comment was marked as outdated.

Sign in to view

@hweihwang
Copy link
Contributor Author

hweihwang commented Dec 2, 2024

Thank you @enjeck! That's a really good points, I didn't think about that actually. I think we can hide those shares when circles disabled, that would be better and more appropriate for this case. Let me know if we expect it different like completely remove those shares @juliusknorr @blizzz , thank you!

@juliusknorr
Copy link
Member

juliusknorr commented Dec 2, 2024

I think we can hide those shares when circles disabled

Yes, would agree with that. I think we do it the same way in the files app 👍

@hweihwang hweihwang requested a review from enjeck December 6, 2024 07:18
@enjeck
Copy link
Contributor

enjeck commented Dec 9, 2024

I think we can hide those shares when circles disabled

To confirm, in addition to hiding the shares from the sidebar, the tables are not shared with the users in a team?

@hweihwang
Copy link
Contributor Author

hweihwang commented Dec 13, 2024

Yes @enjeck, we check from BE so when getting all tables of a user will exclude those circle's shares

blizzz
blizzz reviewed Dec 17, 2024
View reviewed changes
Copy link
Member

@blizzz blizzz left a comment

Choose a reason for hiding this comment

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

Backendwise looks OK (only nitpicks, not blocking), did not do functional tests so far.

@enjeck enjeck mentioned this pull request Dec 29, 2024
Merged
enjeck
enjeck reviewed Dec 29, 2024
View reviewed changes
@hweihwang hweihwang force-pushed the share-with-teams branch 2 times, most recently from 7d0a65b to ad53267 Compare January 7, 2025 11:46
@hweihwang
share with teams
5dacb82 
Signed-off-by: Hoang Pham <hoangmaths96@gmail.com>
@hweihwang hweihwang requested review from blizzz and enjeck January 7, 2025 11:57
juliusknorr
juliusknorr approved these changes Jan 9, 2025
View reviewed changes
Copy link
Member

@juliusknorr juliusknorr left a comment

Choose a reason for hiding this comment

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

Tested and works

@juliusknorr juliusknorr merged commit 9fb5037 into main Jan 9, 2025
63 checks passed
@juliusknorr juliusknorr deleted the share-with-teams branch January 9, 2025 15:17
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@juliusknorr juliusknorr juliusknorr approved these changes

@enjeck enjeck Awaiting requested review from enjeck enjeck is a code owner

@blizzz blizzz Awaiting requested review from blizzz blizzz is a code owner

Assignees

No one assigned

Labels

2. developing Work in progress enhancement New feature or request

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

Share with Teams

5 participants

@hweihwang @juliusknorr @enjeck @blizzz