feat(squads): PromoteToSubCaptain / demote (#358)

[guisaliba]

Closes #358

Parent: #342

What changed

• PromoteToSubCaptain::handle() agora registra somente Member -> SubCaptain.
• PromoteToSubCaptain::demote() agora registra somente SubCaptain -> Member.
• Somente o capitão da própria squad ou um super-admin pode promover ou rebaixar subcapitães.
• Transições com a origem ativa incorreta lançam InvalidSquadRoleTransition.
• Chamadas no estado final esperado continuam idempotentes e não criam evento.
• Cada método público fornece o MembershipAction explicitamente; o fallback genérico de actionFor() foi removido.
• A Action não altera mais a cadeira de capitão. AssignCaptain registra atribuição/substituição e MarkExMember pode deixar a cadeira vaga.
• Mais de um SubCaptain continua permitido, pois não existe requisito de produto nem invariante de schema que limite essa quantidade.
• SquadPolicy::canManage() continua permitindo as capacidades gerais de capitão/subcapitão; a nova capacidade específica exige capitão ou super-admin.

Concurrency

O lock da linha do subject serializa PromoteToSubCaptain e AssignCaptain quando os dois operam sobre a mesma pessoa. Com a validação estrita de origem, uma promoção atrasada que encontra Captain falha com InvalidSquadRoleTransition.

Em pessoas diferentes, esta Action não disputa o índice parcial de capitão porque nunca grava Captain. A corrida real está entre mutações da cadeira de capitão, como dois AssignCaptain concorrentes ou AssignCaptain contra MarkExMember. O conserto por lock da squad foi separado em #527.

Verification

• vendor/bin/pest --compact app-modules/squads/tests/Feature/PromoteToSubCaptainTest.php: 16 testes, 42 assertions.
• vendor/bin/pest --compact app-modules/squads/tests/Feature/SquadPolicyTest.php: 10 testes, 12 assertions.
• vendor/bin/pest --compact app-modules/squads/tests: 61 testes, 132 assertions.
• make test: 987 testes, 3094 assertions.
• make test-pint: passou.
• make test-rector: passou, sem alterações.
• make phpstan: passou, sem erros.
• git diff --check: passou.

[guisaliba]

@gvieira18 @hefeus os dois comentários foram tratados (cada thread respondida e resolvida):

1. @hefeus — sub-capitão movendo o capitão: aceito e corrigido em 305e0e2a. transition() agora bloqueia qualquer transição sobre a linha com role captain quando o ator não é super-admin nem o próprio capitão. Dois testes novos (demote() e handle() contra o capitão) cobrem a regressão; self-step-down (tipo um auto-demote) e override admin seguem válidos.

2. @gvieira18 — nome/padrão da Action: rejeitada a renomeação, com justificativa baseada na spec: PRD prd(squads): squad lifecycle, membership & governance as record-keeping #342 e CONTEXT.md. Ambas listam PromoteToSubCaptain como action canônica do módulo (nome herdado pela feat(squads): PromoteToSubCaptain / demote #358), e o padrão de Action aqui é um outcome/concern por classe, ManageSquadStatus já concentra todo o ciclo de vida num handle só, e AssignCaptain já casa assign+demote internamente. Adicionei um docblock em 15586607 documentando isso para não gerar essa interpretação de novo. Valeu demais pelo questionamento.

Verificação: suite completa verde — 970 testes / 3049 assertions no Pest, pint/rector/phpstan limpos. Podem bater o olho de novo, por favor?

[sirelves]

Fui ler com calma porque essa Action mexe em quem manda na squad. Segue o que achei.

Primeiro o que me deixou tranquilo: o lockForUpdate() dentro da transaction resolve corrida de verdade, e o comentário explicando por que o lock fica na linha do subject me poupou uns dez minutos de leitura. O early return quando o role já é o desejado evita evento duplicado na auditoria, que é o tipo de coisa que só dói meses depois. E os testes cobrem os guards do capitão nos dois sentidos.

Tenho dois pontos que queria conversar antes de aprovar.

1. A squad pode ficar sem capitão, e o módulo diz que isso não acontece

O demote() permite Captain -> Member e o handle() permite Captain -> SubCaptain. Entendi que é intencional, está no corpo do PR. Só que o docblock do AssignCaptain diz o contrário:

Vacating the seat is not this Action's job — a captain who leaves becomes an ExMember (see MarkExMember), which frees the seat by itself.

Até agora o assento só vagava quando o capitão saía da squad. Com este PR ele pode largar o posto e continuar dentro como membro comum. O índice UNIQUE (squad_id) WHERE role = 'captain' garante no máximo um capitão, nunca pelo menos um.

É isso que a gente quer? Se for, vale atualizar o docblock do AssignCaptain, senão ficam duas explicações se contradizendo no mesmo módulo. Se não for, falta alguém assumir o posto na mesma transação.

2. Sub-capitão pode rebaixar sub-capitão

A SquadPolicy trata Captain e SubCaptain como iguais no canManage(), e a Action só protege o assento do capitão. Então um sub-capitão consegue rebaixar outro sub-capitão para Member. Não achei teste cobrindo esse caso.

É proposital? Se a ideia é que só o capitão ou um admin desfaça uma sub-capitania, dá para resolver com um guard parecido com o que já existe para o assento.

Coisas menores, ignora se discordar

O actionFor() trata tudo que não é Member -> SubCaptain como demote. Funciona, porque transition() é privado e só recebe duas roles, mas um match das quatro transições deixaria explícito e aguentaria uma role nova sem virar evento errado na auditoria.

O handle(), que se chama promoção, aceita rebaixar um capitão para sub-capitão. Funciona, mas o nome esconde.

E uma dúvida de concorrência: o lock é na linha do subject, então esta Action e o AssignCaptain podem rodar ao mesmo tempo em pessoas diferentes. O índice parcial impede dois capitães, mas o que chega no usuário nesse caso é uma QueryException de unique violation. Tem tratamento em algum lugar ou a gente aceita?

[guisaliba]

@sirelves

Até agora o assento só vagava quando o capitão saía da squad. Com este PR ele pode largar o posto e continuar dentro como membro comum. O índice UNIQUE (squad_id) WHERE role = 'captain' garante no máximo um capitão, nunca pelo menos um.

É isso que a gente quer? Se for, vale atualizar o docblock do AssignCaptain, senão ficam duas explicações se contradizendo no mesmo módulo. Se não for, falta alguém assumir o posto na mesma transação.

Entendo que não. Esse lock de comportamento de "só podemos dar demote em um capitão se na mesma operação elegermos um capitão novo" contradiz o mundo real. Sabemos que, pelo fato das eleições de captain e sub-captain acontecerem fora da plataforma, as coisas se tornam imprevisíveis e desordenadas. Nem sempre um squad vai ter tempo, organização ou até mesmo disponibilidade de pessoas pra eleger um novo captain, mas isso não pode impedir esse squad de dar demote no capitão atual.

Imagine que o atual capitão do nosso Squad D (você mesmo, Elves) precisa se afastar das ocupações dele dentro do time, mas o time não entra numa decisão de quem vai ser o substituto ou então nem tem "ninguém a altura" pra ser. Estaríamos criando um deadlock imaginário: nossa implementação impede que algo comum no mundo real aconteça, que é afastar alguém de um cargo e esperar por uma decisão de quem será o novo ocupante da cadeira.

Por isso, atualizar a docstring é válido.

A SquadPolicy trata Captain e SubCaptain como iguais no canManage(), e a Action só protege o assento do capitão. Então um sub-capitão consegue rebaixar outro sub-capitão para Member. Não achei teste cobrindo esse caso.

É proposital?

Cada squad só pode ter (max.) um capitão e um sub-capitão. A Action permitir que um sub-capitão consiga rebaixar outro sub-capitão na verdade seria apenas self-demote. Mas entendo que faz MUITO mais sentido um capitão rebaixar um sub (ou um admin, caso necessário). Isso é parte do domínio do jogo: existe uma hierarquia, onde o sub-captain responde diretamente ao captain (e inclusive, substitui este caso ele fique off por um tempo e não possa tomar conta dos seus marujos). Nada mais certo e intuitivo que somente o capitão seja capaz de rebaixar um sub-capitão. Válido um ajuste aqui.

P.S: O próprio BDD da issue já previa isso, então é um erro mesmo.

  Cenário: Rebaixar subcapitão
    Dado um subcapitão
    Quando o capitão o rebaixa
    Então ele volta a member
    E um evento demoted é gravado

Quanto às "coisas menores", uma por uma:

1. Enxergo como uma melhoria, de fato. Hoje, tratar tudo que não é Member -> SubCaptain como demote faz total sentido, mas futuramente isso pode (e se pá vai) mudar e aí pode ser que isso não seja uma verdade absoluta. Válida a implementação disso.
2. Funciona, mas é contraditório. Promote deveria ser apenas promoção, não deveria aceitar uam demoção. Então nada mais justo do que dar o tratamento adequado pra esse cenário, barrando-o.
3. Pra mim indiscutivelmente um bug de concorrência. Não acho que é uma "coisa menor" inclusive. Ao mesmo tempo, não sei como corrigir isso. O lock deveria ser aonde, se não na row do subject? Preciso pensar mais, não sei responder isso agora.

[guisaliba]

Tomei um tempo pra pensar melhor em todos esses pontos. Corrigindo de forma transparente alguns pontos da minha resposta anterior:

1. A vaga de capitão continua válida, mas agora somente pelo fluxo já implementado em MarkExMember. A feat(squads): PromoteToSubCaptain / demote #358 não altera mais linhas com role Captain, então o docblock de AssignCaptain não fica mais em contradição com esta Action.

2. A autoridade ficou específica para esta capacidade. Somente o capitão da própria squad ou um super-admin pode promover ou rebaixar subcapitães. Um subcapitão não pode mais promover membro, rebaixar outro subcapitão ou fazer self-demote. O SquadPolicy::canManage() amplo foi preservado para as outras capacidades que ainda permitem capitão/subcapitão.

3. O fallback de actionFor() foi removido. handle() fornece MembershipAction::Promote e demote() fornece MembershipAction::Demote explicitamente, então uma role futura não cai silenciosamente em demote.

4. handle() agora é somente promoção: aceita Member -> SubCaptain. demote() aceita somente SubCaptain -> Member. Uma origem ativa diferente, como Captain, lança InvalidSquadRoleTransition e não altera estado nem auditoria. Chamadas no estado final esperado continuam sendo no-op sem evento.

5. Eu também corrigi uma afirmação errada da resposta anterior: o PRD e o schema não limitam a squad a um único subcapitão. O índice parcial único existe apenas para Captain. Portanto, múltiplos SubCaptain continuam permitidos.

6. Sobre concorrência, o caso desta Action ficou mais preciso: quando PromoteToSubCaptain e AssignCaptain operam sobre a mesma pessoa, o lock da linha do subject serializa as duas operações; se a promoção atrasada encontrar Captain, ela falha com InvalidSquadRoleTransition. Em pessoas diferentes, esta Action nunca grava Captain e não disputa o índice parcial da cadeira. A corrida real está entre mutações da cadeira de capitão, como dois AssignCaptain concorrentes ou AssignCaptain contra MarkExMember. Criei uma issue separada desta pra tratar disso: fix(squads): serialize captain-seat mutations per squad #527.

As correções estão em fea823b0; a documentação do domínio está em 62ab5257. Localmente passaram 987 testes / 3094 assertions, Pint, Rector e PHPStan. O CI da PR também está todo verde.

Quando puder, pode dar uma revisada de novo? @sirelves