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: 🍰 Post In Groups #5380

Merged
merged 34 commits into from
Oct 11, 2022
Merged

feat: 🍰 Post In Groups #5380

merged 34 commits into from
Oct 11, 2022

Conversation

Mogge
Copy link
Contributor

@Mogge Mogge commented Sep 20, 2022

🍰 Pull Request

Post in groups.

  • add groupId parameter to create post mutation
  • add relation :IN to post in group
  • add relation :CANNOT_SEE to all posts a user cannot see due to group restrictions.

relates #5385

@Mogge Mogge changed the base branch from master to 5059-epic-groups September 20, 2022 18:28
@Tirokk Tirokk changed the title feat [WIP]: Post in Groups feat: [WIP] 🍰 Post In Groups Sep 22, 2022
@Tirokk Tirokk marked this pull request as draft September 22, 2022 06:11
@Mogge Mogge changed the title feat: [WIP] 🍰 Post In Groups feat: 🍰 Post In Groups Oct 5, 2022
@Mogge Mogge marked this pull request as ready for review October 5, 2022 09:30
Copy link
Member

@Tirokk Tirokk left a comment

Choose a reason for hiding this comment

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

Hey @Mogge ,
now we get to the center of our efforts!

I did the first part of my review. I have reviewed until first half of postsInGroups.spec.js.
I'll do the next part later.
So I don't await that this is done if I go ahead.

I have some suggestions and wrote down some of my understandings …

May you like to seed some posts in the groups?
That would be really cool for our stagings and for the development of the webapp part.

Do you like to merge this first or do we do a compare of the webapp part to your code before we merge it to the groups branch?

PS:

Comments of my understanding and the transparence of it for you to check if I'm right:
(more of them you'll find in the code…)

  • if I'm joining a group via JoinGroup no CANNOT_SEE relation changes, because at the moment:
    • in public groups I can see all post already before joining and getting a usual member
      • therefore I can still see all posts
    • in closed groups I can not see the posts before joining and getting a pending member
      • therefore I can still see no posts
    • hidden groups I cannot join this way

backend/src/schema/resolvers/groups.js Outdated Show resolved Hide resolved
backend/src/schema/resolvers/groups.js Outdated Show resolved Hide resolved
backend/src/schema/resolvers/posts.js Show resolved Hide resolved
backend/src/schema/resolvers/posts.js Show resolved Hide resolved
backend/src/schema/resolvers/postsInGroups.spec.js Outdated Show resolved Hide resolved
backend/src/schema/resolvers/postsInGroups.spec.js Outdated Show resolved Hide resolved
backend/src/schema/resolvers/postsInGroups.spec.js Outdated Show resolved Hide resolved
backend/src/schema/resolvers/postsInGroups.spec.js Outdated Show resolved Hide resolved
Co-authored-by: Wolfgang Huß <wolle.huss@pjannto.com>
Copy link
Member

@Tirokk Tirokk left a comment

Choose a reason for hiding this comment

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

Really good stuff dear.

I found some additional things in the masses of tests … 😅
But not a lot … 😁

backend/src/schema/resolvers/postsInGroups.spec.js Outdated Show resolved Hide resolved
backend/src/schema/resolvers/postsInGroups.spec.js Outdated Show resolved Hide resolved
backend/src/schema/resolvers/postsInGroups.spec.js Outdated Show resolved Hide resolved
@Tirokk Tirokk mentioned this pull request Oct 7, 2022
2 tasks
Copy link
Member

@Tirokk Tirokk left a comment

Choose a reason for hiding this comment

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

Mostly works well, but:

  • we may have one misunderstanding
  • and your reverted a change you already did

backend/src/schema/resolvers/groups.js Outdated Show resolved Hide resolved
backend/src/schema/resolvers/groups.js Outdated Show resolved Hide resolved
backend/src/schema/resolvers/posts.js Show resolved Hide resolved
Copy link
Member

@Tirokk Tirokk left a comment

Choose a reason for hiding this comment

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

Hey @Mogge ,
super cool !!! 😍💥

I approve! 🚀🚀🚀💫💫💫

@Mogge Mogge merged commit 51ce290 into 5059-epic-groups Oct 11, 2022
@Mogge Mogge deleted the post-in-group branch October 11, 2022 16:41
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.

2 participants