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

Create intermediate user for channels #45

Open
wants to merge 2 commits into
base: master
Choose a base branch
from
Open

Conversation

coltoneshaw
Copy link
Member

Summary

We sort channels by membership here, which looks at the membership from the slack channel and we append that to the private channels.

Then inside of the TransformChannels function we add the P type ONLY if it has > 8 valid members, here. Which in my case, not all of these channels do have valid members so that check then fails.

Then to complicate the issue, the user is created at the very end as deleted user because they own a post. So, we have these users that own posts in channels, but they're not channel members and contain no membership except town-square.


The PR here might not have a perfect solution. However, I can confirm it resolves the problem within the import and no more channels are imported as type G, and all users (even invalid users) are now properly added to the channel membership.

@coltoneshaw
Copy link
Member Author

I've broken the TestTransformPublicChannelsWithAnInvalidMember test. So, the question here I guess is, what is the expected answer?

In my mind, we need to change and build membership properly for invalid members for the reasons listed above

services/slack/intermediate_test.go Outdated Show resolved Hide resolved
@coltoneshaw
Copy link
Member Author

@mickmister - How do you think we should handle TestTransformPublicChannelsWithAnInvalidMember now?

@mickmister
Copy link
Contributor

mickmister commented Mar 14, 2024

@coltoneshaw Is it the case that a previously "invalid" member is now "valid"? I suppose the test cases in that test function need to be adapted to take into account any new assumptions about data validity. Maybe some test cases need to be removed

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

Successfully merging this pull request may close these issues.

2 participants