Code Review — Task #19323: Stepper condicional no fluxo de venda

Data: 2026-06-03
App: 194760/coezzion_vendas_app
Requisitos: task-19323-update-componente-step-fluxo-venda-requisitos.md

---

Arquivos alterados

Arquivo	Tipo	Linhas
lib/shared/enum/stepp_sell.dart	Modificado	+1 / -20
lib/models/stepper/stepper.dart	Modificado	+11 / -1
lib/controllers/sell/new_sell/upsell_recommendation_controller.dart	Novo	+82
lib/components/get_it.dart	Modificado	+2
lib/screens/sell/new_sell/widgets/new_sell_stepper.dart	Modificado	+75 / -9
7 telas (call sites)	Modificado	+6 / -7
test/controllers/sell/new_sell/upsell_recommendation_controller_test.dart	Novo	+127
test/models/stepper/stepper_model_test.dart	Novo	+40

---

Review por componente

1. stepp_sell.dart — Enum + remoção de listas mutáveis globais

Avaliação: ✅ Correto

A adição do valor recomendar ao enum SteppSell é exatamente o previsto no RF-06.

A remoção das três listas mutáveis globais (stepperSellProduct, stepperSellCustomer, stepperSellCheck) é uma boa refatoração — essas listas eram um anti-padrão (estado global mutável representando status de UI). Com a nova abordagem, o NewSellStepper calcula os statuses internamente com base no currentStep. Isso alinha com SRP e melhora testabilidade.

---

2. stepper.dart — Novos factories

Avaliação: ✅ Correto — [RF-06]

Factory	Parâmetros	Comportamento
recomendaStepperModel(status)	StatusStepp status	number: 3, title: 'Recomendar', steppSell: SteppSell.recomendar
conferenciaStepperModel(status, {number = 3})	StatusStepp status, {int number}	Number dinâmico com default 3; quando upsell ativo → 4

A decisão de usar parâmetro nomeado opcional ({int number = 3}) em vez de sobrecarga é idiomática no Dart e mantém retrocompatibilidade com callers que não precisam do parâmetro.

---

3. upsell_recommendation_controller.dart — Controller dedicado

Avaliação: ✅ Muito bom — [RF-02, RF-03, RF-04]

Pontos fortes:

• Lógica pura em evaluateEligibility(): método síncrono, 100% testável, sem dependências de framework. As 3 condições (feature flag global, whitelist de lojas, saleEcommerce == false) são avaliadas com AND em early-return — eficiente e legível.

• @visibleForTesting lambdas: abordagem pragmática para injeção de dependências nos testes, evitando mocks complexos do FirebaseRemoteConfig e StoreService. Em produção os lambdas acessam os serviços reais estaticamente.

• Caso de borda — whitelist vazia ou strings vazias: linha 66–77:

  if (allowedStores.isEmpty || allowedStores.every((s) => s.isEmpty)) {
    return false;
  }

  Cobre o cenário de whitelist não configurada ou mal formatada no Firebase Remote Config. ✅

• Listener reativo via activeCartDTO: o StreamSubscription é criado em onInit e cancelado em onClose. Como o controller é lazySingleton, o listener sobrevive entre telas — exatamente o comportamento esperado.

Pontos de atenção:

• 🟡 Linha 78–79 — getCurrentStoreCode pode retornar null: se StoreService.activeStore for null (ex: loja ainda não selecionada), retorna false — comportamento seguro, consistente com RF-02.

• 🔵 Info: Não há tratamento do requisito "step permanece visível durante o uso mesmo se feature flag mudar em runtime" (RF-03, critério 4). Esse requisito é conflitante com a reatividade automática — o controller reavalia a cada emissão do activeCartDTO. Se a flag mudar enquanto o usuário está na tela de recomendação, o step desaparecerá. Isso pode ser tratado como escopo de outra task (ex: a própria tela de recomendação gerencia esse estado local), mas vale confirmar com o PO.

---

4. new_sell_stepper.dart — Refatoração principal

Avaliação: ✅ Bom, com 2 ressalvas — [RF-05]

Mudança de API:

- NewSellStepper(List<StatusStepp> listStatus)
+ NewSellStepper({required SteppSell currentStep})

A responsabilidade de calcular os statuses dos steps mudou dos callers para dentro do widget. Isso reduz código nos 7 call sites e centraliza a lógica de renderização condicional.

Estrutura reativa:

Widget build(BuildContext context) {
  final controller = _getController();
  if (controller == null) return _buildStepper(isActive: false);
  return AppGetBuilder<UpsellRecommendationController>(
    init: controller,
    builder: () => _buildStepper(isActive: controller.isActive),
  );
}

Correto: usa AppGetBuilder (padrão do projeto), fallback para isActive: false se controller não estiver disponível, e o builder reconstrói apenas a sub-árvore dos steps — não o widget inteiro.

_buildStepModels — lógica de geração da lista de steps:

final allSteps = isActive
    ? [SteppSell.produto, SteppSell.cliente, SteppSell.recomendar, SteppSell.conferencia]
    : [SteppSell.produto, SteppSell.cliente, SteppSell.conferencia];
final currentIndex = allSteps.indexOf(currentStep);

A computação de status via index < currentIndex → complete, index == currentIndex → selected, index > currentIndex → unselected é correta para os índices normais.

🟡 Ressalva: Se currentStep == SteppSell.recomendar mas isActive == false (possível em race condition onde a tela foi aberta mas a flag mudou antes do rebuild), indexOf retorna -1. Nesse cenário, todos os steps ficariam unselected (já que 0 < -1 é falso e 0 == -1 é falso). Visualmente o stepper mostraria Produtos, Cliente e Conferir todos no estado unselected, sem step ativo — comportamento degradado aceitável, mas inconsistente.

🟡 Ressalva: _getController() usa try/catch defensivo:

try {
  return getIt.get<UpsellRecommendationController>();
} catch (_) {
  return null;
}

O controller é registrado como registerLazySingleton em get_it.dart. Se initGetIt() foi chamado (garantido no boot do app), esse get nunca deveria lançar. O try/catch pode mascarar erros de configuração de DI que deveriam ser visíveis em desenvolvimento. Sugestão: adicionar um debugPrint ou assert no catch em modo debug.

_interleaveDividers — insere _SteppDivider entre cada par de steps:

List<Widget> _interleaveDividers(List<StepperModel> steps) {
  final widgets = <Widget>[];
  for (var i = 0; i < steps.length; i++) {
    widgets.add(_ItemStepp(steps[i]));
    if (i < steps.length - 1) {
      widgets.add(const _SteppDivider());
    }
  }
  return widgets;
}

Implementação limpa, funciona tanto para 3 quanto para 4 steps. ✅

---

5. Call sites — 7 telas migradas

Avaliação: ✅ Migração limpa — [RF-07]

Tela	Antes	Depois
NewSellScreen	newSellType == product || customerProfile ? stepperSellProduct : stepperSellCustomer	currentStep: newSellType == product || customerProfile ? SteppSell.produto : SteppSell.cliente
CheckSellScreen	stepperSellCheck	currentStep: SteppSell.conferencia
AddCustomerScreen	stepperSellCustomer	currentStep: SteppSell.cliente
CheckAddressCustomerScreen	stepperSellCustomer	currentStep: SteppSell.cliente
CheckCustomerScreen	stepperSellCustomer	currentStep: SteppSell.cliente
SelectDeliveryMethodScreen	stepperSellCustomer	currentStep: SteppSell.cliente
AddProductsScreen	stepperSellProduct	currentStep: SteppSell.produto
CheckProductsScreen	stepperSellProduct	currentStep: SteppSell.produto

Todas as substituições são corretas. Nenhum call site precisou adicionar wrapper AppGetBuilder — a reatividade está encapsulada no próprio NewSellStepper (RF-07, critério 4). ✅

---

6. get_it.dart — DI

Avaliação: ✅ Correto

registerLazySingleton(() => UpsellRecommendationController());

Registrado como lazySingleton logo após a importação do controller. Como é um lazySingleton, só será instanciado no primeiro acesso via getIt.get<UpsellRecommendationController>() — isso ocorre dentro do build() do NewSellStepper. O posicionamento no arquivo está adequado.

---

7. Testes unitários

Avaliação: ✅ Cobertura abrangente

upsell_recommendation_controller_test.dart — 8 cenários

Teste	Cobertura
cart null → false	RF-02 borda
saleEcommerce null → false	RF-02 borda
saleEcommerce true → false	RF-02 critério 1
saleEcommerce false + flag false → false	RF-02 critério 2
nome na whitelist → false	RF-02 borda
whitelist vazia → false	borda adicional
whitelist com strings vazias → false	borda adicional
todas condições true → true	RF-02 critério 1
activeStore null → false	borda adicional

Pontos fortes:

• Usa Get.testMode = true (setup/teardown correto)
• Injeta lambdas via @visibleForTesting — sem necessidade de mock complexo
• Cobre whitelist vazia e strings vazias (casos de borda de configuração incorreta do Remote Config)

🔵 Falta: Testes de integração do listener (_onCartChanged, onInit/onClose). O evaluateEligibility está muito bem testado, mas o comportamento reativo (subscription no activeCartDTO) não tem cobertura. Para um controller test, seria valioso simular emissões de stream e verificar que update() é chamado e isActive muda.

stepper_model_test.dart — 3 cenários

Teste	Cobertura
recomendaStepperModel → number=3, title='Recomendar'	RF-06 critério 2
conferenciaStepperModel sem number → number=3	RF-06 critério 3 (default)
conferenciaStepperModel com number: 4 → number=4	RF-06 critério 3 (upsell ativo)

Cobertura correta para os requisitos. ✅

🔵 Falta: Não há widget test para NewSellStepper. O widget passou de um renderizador estático de 3 steps para um componente reativo condicional com AppGetBuilder — um widget test que verifica a renderização com isActive: true/false e diferentes currentStep aumentaria a segurança da refatoração.

---

Sumário de achados

#	Severidade	Arquivo	Descrição
1	🟡 Média	new_sell_stepper.dart:_buildStepModels	currentStep == recomendar com isActive == false → indexOf retorna -1 → todos steps unselected
2	🟡 Média	upsell_recommendation_controller.dart	RF-03 critério 4 ("step não desaparece durante uso") conflita com reatividade automática — não implementado
3	🔵 Baixa	new_sell_stepper.dart:_getController	try/catch silencioso — adicionar debugPrint ou assert em modo debug
4	🔵 Baixa	Testes	Sem cobertura de integração do stream listener no controller
5	🔵 Baixa	Testes	Sem widget test para NewSellStepper
6	ℹ️ Info	.DS_Store untracked	Deve permanecer fora do stage ou ser adicionado ao .gitignore

---

Conclusão

A implementação segue rigorosamente os requisitos EARS (RF-01 a RF-07), mantém as convenções do projeto (StatelessWidget + AppGetBuilder + lazySingleton), tem testes unitários com boa cobertura de borda, e a migração dos 8 call sites é limpa e sem regressões.

Os achados #1 e #2 merecem atenção antes de merge para produção — #1 pode ser resolvido com uma guard clause de ~3 linhas, e #2 requer alinhamento com PO sobre o comportamento esperado quando a feature flag muda em runtime.

Os achados #3–#5 são melhorias de qualidade desejáveis mas não bloqueadores. Recomendo criar tasks de follow-up para os testes de widget e de integração do stream.