artsy / positron

Positron is Artsy Writer or the editorial tool and API for Artsy.
MIT License
85 stars 42 forks source link

feat: replace team role with editorial role #3126

Closed egdbear closed 8 months ago

egdbear commented 9 months ago

This pull request removes the 'team' and 'admin' roles and replaces them with the 'editorial' role.

DIA-230

Gravity PR for enabling the Editorial roles

@artsy/diamond-devs

codecov[bot] commented 9 months ago

Codecov Report

Attention: 2 lines in your changes are missing coverage. Please review.

Comparison is base (6393e4e) 83.6% compared to head (7876fd3) 83.6%.

Additional details and impacted files ```diff @@ Coverage Diff @@ ## main #3126 +/- ## ===================================== Coverage 83.6% 83.6% ===================================== Files 197 197 Lines 5375 5378 +3 Branches 976 977 +1 ===================================== + Hits 4496 4499 +3 Misses 626 626 Partials 253 253 ```
egdbear commented 9 months ago

Suggested migration: This will block Positron's existing users from certain actions unless we grant them the new role prior to releasing this. We can query recent log-ins (like here) to identify the accounts that should be considered for the new role, then confirm with the Editorial folks.

I was wondering where to add those emails. I will add the account emails to the ticket.

Potential bug: Some of these admin-only cases may depend on back-end APIs that also require the admin role, like to search users. Does Gravity need changes to support the new role there?

/search - under the hood is calling an ES endpoint. I believe we don't have any authorization set for that?.

/api/v1/me - is being called when using the /users endpoint.

egdbear commented 9 months ago

Possibly because M1, asdf and outdated SSL (due to old ruby versions) I couldn't run the app.

To run locally connected to staging I had to update the scripts/start.sh

# !/usr/bin/bash

set -e -x

forever -c 'node --max_old_space_size=1024 --no-experimental-fetch --openssl-legacy-provider' ./src/index.js --colors
joeyAghion commented 9 months ago

I haven't followed the latest changes closely enough to know when this can be merged, so I'll approve and add you to assignees. Let me know if you can use more feedback!

mc-jones commented 8 months ago

🙏