Bulk command update API design can be improved to avoid footguns

[MinnDevelopment]

Right now, all commands of all types are sent as one big list to the bulk update endpoint. This becomes problematic when new types are added that a library doesn't support. In that case, the library will simply delete all of these commands unknowingly, resulting in unexpected footguns.

This has re-appeared recently with the new entry point command types 4, resulting in this band-aid error:

50240: You cannot remove this app's Entry Point command in a bulk update operation. Please include the Entry Point command in your update request or delete it separately.

The only reason why this error seems to exist, is because API users are unaware about the connection between activities and other command types. Libraries, naturally, haven't implemented these types yet and thus also are entirely unaware.

Instead of implementing new types by just adding them to the same list, they should be managed as separate lists.

An example:

PUT /api/applications/123456/commands

{
  "slash": [...],
  "message": [...],
  ...
}

This can be made forward-compatible, by allowing each list to be omittable, in which case commands of those types are not changed. If a library doesn't support a specific new type of command, it will simply not send the list for that type. This allows a library to continue to function in parallel to new command types being added for various features, and removes the need for errors like shown above entirely.

[ckohen]

I'd be inclined to say this should be an array of { type: CommandType, commands: Command[] } rather than an object. This is more flexible and probably easier to implement?

for (const commandSet of commandSets) {
  sql(`DELETE FROM Commands WHERE Type = ${commandSet.type};`);
  const commandsAsValues = commandSet.commands.map(command => `(${command.type}, ${command.name})`).join(',');
  sql(`INSERT INTO Commands (Type, Name /* etc... */) VALUES ${commandsAsValues};`);
}

Obviouslly missing assertion checks and there's a better way to map the commands internals to the database, but this shows it somewhat simply.

[MinnDevelopment]

That would also work. Another possibility is just listing the affected types separately:

{
  "known_types": [1, 2, 3],
  "commands": [ ... ]
}

[Zoddo]

IMO, it would be better to list known_types in a query parameter, to not introduce another breaking change by changing the type of the body payload (array of commands to a json object).

PUT /api/applications/123456/commands?types=1,2,3
If types is omitted, it should update every existing type (current behavior). Or even better, default to the currently documented type (1,2,3), so it's 100% backward compatible with non-updated libs, even if activities are enabled.