greenlight icon indicating copy to clipboard operation
greenlight copied to clipboard

Remove admin::user#destroy endpoint duplication + "admin" specs template proposal

Open scouillard opened this issue 2 years ago • 4 comments

Description

Remove the duplicated user#destroy endpoint.

Propose a new template for "admin" specs: In specs, it can be difficult to differentiate regular user specs from "admin" users specs.

  1. The roles/permissions are set on top of the resource spec files (this can be moved in another module eventually)
  let(:<permission>_role) { create(:role) }
  let(:<permission>_permission) { create(:permission, name: '<permission>') }
  let(:<permission>_role_permission) do
    create(:role_permission,
           role: <permission>_role,
           permission: <permission>_permission,
           value: 'true',
           provider: 'greenlight')
  end
  1. A context is created inside the describe <endpoint>
describe 'some#endpoint' do
  it 'does something' do 
    test something
  end

  context 'user with <permission> is doing something' do
     before do 
       <permission>_role_permission ## instantiate the role_permission
       user.update!(role: <permission>_role) ## assign current_user the permission
     end

     it 'does something' do 
       test something
     end 
end

scouillard avatar Aug 11 '22 19:08 scouillard

The only issue that I see now is that some test files have the case where most of their tests require a permission (such as roles_controller_spec). In this case I find it cleaner in a way to just give the user the permission at the beginning and then disable it where needed.

I feel your proposal works best in the cases where majority of the tests in the file don't need the permission being tested.

^ That being said, I'm not sure what the best approach would be atm, but just thought I should share my thinking

hadicheaito1 avatar Aug 12 '22 19:08 hadicheaito1

The only issue that I see now is that some test files have the case where most of their tests require a permission (such as roles_controller_spec). In this case I find it cleaner in a way to just give the user the permission at the beginning and then disable it where needed.

I feel your proposal works best in the cases where majority of the tests in the file don't need the permission being tested.

^ That being said, I'm not sure what the best approach would be atm, but just thought I should share my thinking

Absolutely - in those cases, you would do the opposite.

In most Specs tho I think this template will work great.

scouillard avatar Aug 12 '22 19:08 scouillard

It was obvious after two days of trying to refactor the Specs that something needed to be done for the user "permissions" specs. I like working with this template, and I hope you guys like it too. Of course this solution can be improved or even replaced.

scouillard avatar Aug 12 '22 19:08 scouillard

SonarCloud Quality Gate failed.    Quality Gate failed

Bug C 13 Bugs
Vulnerability A 0 Vulnerabilities
Security Hotspot A 0 Security Hotspots
Code Smell A 87 Code Smells

No Coverage information No Coverage information
1.5% 1.5% Duplication

sonarqubecloud[bot] avatar Aug 16 '22 17:08 sonarqubecloud[bot]