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.

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

[BREAK] Freeze Custom Roles #27481

Merged
merged 15 commits into from
Feb 7, 2023
Merged

[BREAK] Freeze Custom Roles #27481

merged 15 commits into from
Feb 7, 2023

Conversation

LucianoPierdona
Copy link
Contributor

@LucianoPierdona LucianoPierdona commented Dec 7, 2022 •
edited
Loading

Proposed changes (including videos or screenshots)

This PR moves role creation and update to enterprise folder

--

  • Removed method authorization:saveRole
  • Moved /v1/roles.create/ and /v1/roles.update to enterprise edition
  • Changed message for delete a custom role in community edition

Issue(s)

Steps to test or reproduce

Further comments

TC-282

@LucianoPierdona LucianoPierdona added this to the 6.0.0 milestone Dec 7, 2022
@LucianoPierdona LucianoPierdona requested a review from a team as a code owner December 7, 2022 14:25
@LucianoPierdona LucianoPierdona self-assigned this Dec 7, 2022
@LucianoPierdona LucianoPierdona marked this pull request as draft December 7, 2022 20:27
@codecov
Copy link

codecov bot commented Dec 8, 2022 •
edited
Loading

Codecov Report

Merging #27481 (8013652) into develop (cdc2f99) will increase coverage by 1.02%.
The diff coverage is 100.00%.

Impacted file tree graph

@@             Coverage Diff             @@
##           develop   #27481      +/-   ##
===========================================
+ Coverage    40.94%   41.97%   +1.02%     
===========================================
  Files          838      847       +9     
  Lines        17677    17550     -127     
  Branches      1983     2078      +95     
===========================================
+ Hits          7238     7366     +128     
+ Misses       10201     9904     -297     
- Partials       238      280      +42
Flag Coverage Δ
e2e 41.97% <100.00%> (+1.02%) ⬆️

Flags with carried forward coverage won't be shown. Click here to find out more.

@LucianoPierdona LucianoPierdona marked this pull request as ready for review December 13, 2022 16:37
@LucianoPierdona LucianoPierdona requested a review from a team as a code owner December 13, 2022 16:37
@gabriellsh
Merge branch 'develop' of github.com:RocketChat/Rocket.Chat into feat…
c427899 
…/freeze-custom-roles

* 'develop' of github.com:RocketChat/Rocket.Chat: (67 commits)
  Chore: Reduce Less usage in theming (#27689)
  [IMPROVE] Increase read receipt size (#27665)
  Chore: Improve `ts-node` startup time with `swc` (#27686)
  Fix write stream error on user data export (#27683)
  Regression: Rectifying thread layout (#27680)
  [FIX] Showing Workspace Section For User Having LiveChat Manager Permission (#27188)
  [FIX] Team mention highlight EE (#27493)
  [IMPROVE] Save toPhone when receiving SMSs (#27656)
  Chore: Upgrade EE license for CI (#27682)
  Chore: Replace borders to `default` (#27667)
  Chore: Refactor message subcomponents (#27630)
  Regression: Fix go to ThreadList hook (#27663)
  Chore: Refactor e2e selectors to use roles (#27582)
  Chore: Composer wrapper around the textarea (#27658)
  [FIX] Remove invalid settings at startup (#27653)
  Chore: Token refactor (#27655)
  Regression: Composer fix join room action (#27657)
  Regression: Sidebar collapsing on extra small views (#27651)
  Chore: Composer missing Slash Command preview (#27652)
  Chore: Custom emoji empty state (#27641)
  ...
gabriellsh
gabriellsh previously approved these changes Jan 5, 2023
View reviewed changes
@dionisio-bot dionisio-bot bot added the stat: ready to merge PR tested and approved waiting for merge label Feb 6, 2023
@rodrigok
Copy link
Member

rodrigok commented Feb 7, 2023

@LucianoPierdona great PR. Just need to adjust the description since this removes the method rather than deprecating it

@rodrigok rodrigok merged commit 4a654b7 into develop Feb 7, 2023
@rodrigok rodrigok deleted the feat/freeze-custom-roles branch February 7, 2023 18:46
@danel1
Copy link

danel1 commented Feb 9, 2023 •
edited
Loading

@rodrigok @gabriellsh Are you seriously removing the whole permissions / roles management for all "free" users....?

@rodrigok
Copy link
Member

Hi @danel1

From version 6.0 the Community Edition won't have the ability to create new Custom Roles. Existing Custom Roles will continue working but won't be able to be updated (change name or description), if deleted, they can't be recreated. So the effective change here is the movement of the API to create Custom Roles to the Enterprise Edition.

The ability to set permissions on native Roles, and the Custom Roles that already existed before the upgrade, will continue to be available in Community Edition.

In fact, the only change here is related to Custom Roles, not to the rest of the roles/permission system.

@sampaiodiego sampaiodiego mentioned this pull request Feb 17, 2023
Merged
@sampaiodiego sampaiodiego mentioned this pull request Mar 9, 2023
Merged
13 tasks
@ulope
Copy link

ulope commented Apr 12, 2023

This also breaks the option to enable or disable the Users must use Two Factor Authentication setting.

But sine that setting is broken anyway I guess who the fuck cares.

@ulope ulope mentioned this pull request Apr 13, 2023
Open
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@gabriellsh gabriellsh gabriellsh approved these changes

@rodrigok rodrigok rodrigok approved these changes

@matheusbsilva137 matheusbsilva137 matheusbsilva137 left review comments

Assignees

@LucianoPierdona LucianoPierdona

Labels
squad: team-collab stat: ready to merge PR tested and approved waiting for merge
Projects
None yet
Milestone
6.0.0
Development

Successfully merging this pull request may close these issues.
7 participants
@LucianoPierdona @rodrigok @danel1 @ulope @matheusbsilva137 @gabriellsh @jessicaschelly