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

Issue 592 transfer ownership v2 #731

Merged
merged 31 commits into from Nov 26, 2017

Conversation

Projects
None yet
6 participants
@railsstudent
Collaborator

railsstudent commented Nov 19, 2017

Fixes #592

A new section TRANSFER STUDY GROUP is visible in Settings tab if the group as at least 1 non-owner member
When owner transfer study group to another member, the current owner becomes member of the study group and a new activity is logged to Activity tab
Afterward, old owner is redirected to all study group

transfer_study_group_setting
confirmation_dialog
new_member_status
study_group_activities

@lpatmo lpatmo added the In Review label Nov 19, 2017

@nalbina

This comment has been minimized.

Show comment
Hide comment
@nalbina

nalbina Nov 19, 2017

Collaborator

hey @railsstudent is this WIP?

Collaborator

nalbina commented Nov 19, 2017

hey @railsstudent is this WIP?

@railsstudent

This comment has been minimized.

Show comment
Hide comment
@railsstudent

railsstudent Nov 20, 2017

Collaborator

it is completed @nalbina

Collaborator

railsstudent commented Nov 20, 2017

it is completed @nalbina

@nalbina

This comment has been minimized.

Show comment
Hide comment
@nalbina

nalbina Nov 21, 2017

Collaborator

I am somehow missing the option to transfer the group to another member. are you sure you added the relevant html portion or could you point me to it? :)

Collaborator

nalbina commented Nov 21, 2017

I am somehow missing the option to transfer the group to another member. are you sure you added the relevant html portion or could you point me to it? :)

@railsstudent

This comment has been minimized.

Show comment
Hide comment
@railsstudent

railsstudent Nov 22, 2017

Collaborator

U are right, when i make a new branch and squash commits, i lost the UI changes in study group settings.html. I will add it back tonight. Thanks for checking with me.

Collaborator

railsstudent commented Nov 22, 2017

U are right, when i make a new branch and squash commits, i lost the UI changes in study group settings.html. I will add it back tonight. Thanks for checking with me.

@nalbina nalbina added in progress and removed In Review labels Nov 22, 2017

@railsstudent

This comment has been minimized.

Show comment
Hide comment
@railsstudent

railsstudent Nov 22, 2017

Collaborator

@nalbina Fixed. Please kindly review.
Amendment: After study group is transferred, member remains in the study group and Description tab becomes visible.

Collaborator

railsstudent commented Nov 22, 2017

@nalbina Fixed. Please kindly review.
Amendment: After study group is transferred, member remains in the study group and Description tab becomes visible.

@distalx

Thanks for the PR! Here are some suggestions I hope you'll find helpful.

Show outdated Hide outdated server/study_groups/methods.js Outdated
Show outdated Hide outdated server/study_groups/methods.js Outdated
Show outdated Hide outdated server/study_groups/methods.js Outdated
Show outdated Hide outdated client/templates/study_groups/study_group_settings.html Outdated

railsstudent and others added some commits Nov 22, 2017

Revise permission logic
Demote old owner to admin
Check new owner is admin, member or moderator of the study group
Check caller is owner of the study group
demote owner to admin
Merge branch 'issue-592-transfer-ownership-v2' of git://github.com/ra…
…ilsstudent/codebuddies into railsstudent-issue-592-transfer-ownership-v2
@distalx

This comment has been minimized.

Show comment
Hide comment
@distalx

distalx Nov 26, 2017

Collaborator

Great work @railsstudent 👏

thanks for the review @ajparise, @angelocordon , @nalbina. 💯

Collaborator

distalx commented Nov 26, 2017

Great work @railsstudent 👏

thanks for the review @ajparise, @angelocordon , @nalbina. 💯

distalx added some commits Nov 26, 2017

@distalx distalx merged commit 9f69ca2 into codebuddies:staging Nov 26, 2017

@lpatmo lpatmo added closed and removed in progress labels Nov 26, 2017

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment