[AC-85] Set Max Seats Autoscale and Current Seats via Public API

[cyprain-okeke]

Type of change

- [ ] Bug fix
- [X] New feature development
- [ ] Tech debt (refactoring, code cleanup, dependency upgrades, etc)
- [ ] Build/deploy pipeline (DevOps)
- [ ] Other

Objective

As an organizational admin user of the public API, I want to be able to update max seat autoscaling, current seats, and storage via the Public API so that I do not have to do this manually and so that I can perform this in bulk automatically on a given date each month to combine invoice/billing changes, which will reduce overhead and load on our accounts payable team in dealing with autoscaling invoices as users provision on-demand.

Code changes

• file.ext: Description of what was changed and why

Before you submit

• Please check for formatting errors (dotnet format --verify-no-changes) (required)
• If making database changes - make sure you also update Entity Framework queries and/or migrations
• Please add unit tests where it makes sense to do so (encouraged but not required)
• If this change requires a documentation update - notify the documentation team
• If this change has particular deployment requirements - notify the DevOps team

[bitwarden-bot]

Checkmarx One – Scan Summary & Details – 4d4a6bae-ffaf-4deb-b40b-0d7462d973eb

No New Or Fixed Issues Found

[eliykat]

This is looking good, some feedback below.

One outstanding issue is that this doesn't look like it addresses Jordan's comment here:

On the topic of maxAutoScale, I would expect a null value to leave my value unchanged, and a -1 to disable the seat limit entirely, so that I don't have to perform a separate call to look up what my existing max is set to. In a PUT request, I think it makes sense to leave the value unchanged - I'm not PUTing a value to that object so keep what is already there.

I agree with that suggestion, that is,

• if the value is null, don't change it
• for nullable properties like max autoscale - if the value is -1, set it to null. The idea here is to distinguish between an "intentional null" (clearing the value) and an unintentional null (not supplying a value, therefore leaving it unchanged). Or in Javascript terms, null vs. undefined.

(This is only an issue for properties that are nullable in our database. For non-nullable properties, the user can never update it to null, so we can safely skip it if not provided.)

I'm sure this isn't the only way to deal with this, feel free to see what we're doing elsewhere in our public API at the moment, or what stackoverflow recommends, or what #code thinks.

[withinfocus]

I haven't followed the entire conversation here but would still think a shared controller is possible.

[cturnbull-bitwarden]

Sorry that it took me some time to take this over this PR review from @eliykat! Thank you for your work on this so far. I did leave some comments, however. Feel free to reach out if you'd like to discuss them

[cturnbull-bitwarden]

Nice work!

[eliykat]

I removed my review given that @cturnbull-bitwarden took over for me. Thanks all!

outdated