netlify / open-api

Open API specification of Netlify's API
https://open-api.netlify.com/#/default
MIT License
263 stars 84 forks source link

feat: Replace `Controller` role with `Billing Admin` role #528

Closed Ayushi3333 closed 9 months ago

Ayushi3333 commented 10 months ago

We recently renamed all Controllers to Billing Admins in PR. This needs to be reflected in the api docs.

netlify[bot] commented 10 months ago

Deploy Preview for open-api ready!

Name Link
Latest commit bdb29d0753df66e0fdfa1e51ade8611fb4879f52
Latest deploy log https://app.netlify.com/sites/open-api/deploys/65ce067217074e000888248d
Deploy Preview https://deploy-preview-528--open-api.netlify.app
Preview on mobile
Toggle QR Code...

QR Code

Use your smartphone camera to open QR code link.

To edit notification comments on pull requests, go to your Netlify site configuration.

conventional-commit-lint-gcf[bot] commented 10 months ago

🤖 I detect that the PR title and the commit message differ and there's only one commit. To use the PR title for the commit history, you can use Github's automerge feature with squashing, or use automerge label. Good luck human!

-- conventional-commit-lint bot https://conventionalcommits.org/

Ayushi3333 commented 9 months ago

this seems like a breaking change? should we have a backwards compatible API that allows Controller as an alias for Billing Admin?

also i think you need to run go generate ./... and commit the generated files

Can you elaborate how this is a breaking change? We no longer accept Controller as a role in the API requests. This change is ensuring Controller is no longer used because it won't work. If you are worried about our UI, netlify-react-ui is sending both roles in the params to support the transition.

Thanks for the second suggestion, I will try to generate the files.

mraerino commented 9 months ago

Can you elaborate how this is a breaking change? We no longer accept Controller as a role in the API requests. This change is ensuring Controller is no longer used because it won't work. If you are worried about our UI, netlify-react-ui is sending both roles in the params to support the transition.

yeah, exactly what i meant. we made a breaking change to our API since existing code from customers that might have used Controller now fails. i'm bringing this up here because everything that we document in open-api is supposed to be fairly stable, since once it's documented publicly, customers will start relying on it. if we hadn't documented it, the breaking change wouldn't be too significant since we don't need to provide stability when we expect a method to only be used in our UI.

we have this distinction deliberately. there is an internal open api spec that contains all methods that our API offers, not just the ones we want to provide to customers

Ayushi3333 commented 9 months ago

Can you elaborate how this is a breaking change? We no longer accept Controller as a role in the API requests. This change is ensuring Controller is no longer used because it won't work. If you are worried about our UI, netlify-react-ui is sending both roles in the params to support the transition.

yeah, exactly what i meant. we made a breaking change to our API since existing code from customers that might have used Controller now fails. i'm bringing this up here because everything that we document in open-api is supposed to be fairly stable, since once it's documented publicly, customers will start relying on it. if we hadn't documented it, the breaking change wouldn't be too significant since we don't need to provide stability when we expect a method to only be used in our UI.

we have this distinction deliberately. there is an internal open api spec that contains all methods that our API offers, not just the ones we want to provide to customers

You are absolutely right that this would be a problem if this was being actively used. There are 85 non-netlify Billing Admins all time and I have a strong suspicion they were not created through the API but rather through the UI.

I do not think this role is being created by the users programatically. To substantiate my claim, I tried looking in Humio but I cannot find a way to differentiate "user-generated" requests vs our UI. I can find some data to confirm once I figure out how to obtain it.

EDIT - I found a way to query requests not coming from our FE app and as you can see [here](https://cloud.us.humio.com/netlify-us-production/search?columns=%5B%7B%22type%22%3A%22field%22%2C%22fieldName%22%3A%22%40rawstring%22%2C%22format%22%3A%22logline%22%7D%5D&end=1707997735439&live=false&newestAtBottom=true&query=%24bb()%20%7C%20%22oauth_application_id%22%20!%3D%20bf44ab41f747c09bb9f4b274c0aad99acffc7a613cd69683af7f8787ef90381a%20%7C%20path%3D%22*%2Faccounts%2F*%2Fmembers%22%20%7C%20path%20!%3D%22*hero*%22%20%7C%20method%20!%3D%20GET&showOnlyFirstLine=false&start=1700006400000&tz=Europe%2FLondon&widgetType=list-view), in the last 3 months (our Humio retention period), there's not been a single request to create or update Controller roles. I think it is safe to say users are not using these endpoints programatically and it is safe to update it in the documentation.