diff --git a/app/controllers/api/v1/demandas_controller.rb b/app/controllers/api/v1/demandas_controller.rb index 6531ef9..bb2de3f 100644 --- a/app/controllers/api/v1/demandas_controller.rb +++ b/app/controllers/api/v1/demandas_controller.rb @@ -44,14 +44,32 @@ def update # DELETE /api/v1/demandas/:id # Apenas o líder pode excluir uma demanda já existente. + # + # O 422 não é alcançável hoje — nada impede a exclusão de uma + # demanda. Existe porque antes o 204 era devolvido sem olhar o + # retorno de #destroy: um `dependent: :restrict_with_error` ou um + # `before_destroy` que aborte fariam a API responder "excluída" com + # o registro ainda no banco. Ver o comentário equivalente em + # DemandasController#destroy (tela web). def destroy authorize! :destroy, @demanda - @demanda.destroy - head :no_content + + if @demanda.destroy + head :no_content + else + render json: { errors: erros_de_exclusao }, status: :unprocessable_content + end end private + # Mesmo formato de erro de #create/#update (array de strings). Um + # `before_destroy` com `throw :abort` devolve false sem popular + # errors, daí o fallback — a resposta nunca sai com a lista vazia. + def erros_de_exclusao + @demanda.errors.full_messages.presence || ['Não foi possível excluir a demanda.'] + end + def set_demanda @demanda = Demanda.find(params[:id]) end diff --git a/app/controllers/demandas_controller.rb b/app/controllers/demandas_controller.rb index ff0cdd2..c92107b 100644 --- a/app/controllers/demandas_controller.rb +++ b/app/controllers/demandas_controller.rb @@ -73,14 +73,38 @@ def update end end + # Hoje nada impede a exclusão de uma demanda, então o caminho de erro + # abaixo não é alcançável — e é justamente esse o motivo de ele existir. + # Antes o sucesso era anunciado sem olhar o retorno de #destroy, o que + # estava certo por coincidência, não por verificação: um + # `dependent: :restrict_with_error`, um `before_destroy` que aborte ou + # uma FK nova fariam a tela dizer "excluída com sucesso" com o registro + # ainda no banco. + # + # Contraste com Users::Destroy, que existe porque a exclusão de usuário + # TEM impeditivos (conta própria, demandas vinculadas). Essa diferença + # entre os dois casos não estava registrada em lugar nenhum. Não vale um + # Demandas::Destroy agora: sem regra de negócio para compartilhar entre + # web e API, seria uma classe só para embrulhar uma chamada. def destroy authorize! :destroy, @demanda - @demanda.destroy - redirect_to demandas_path, notice: 'Demanda excluída com sucesso.' + + if @demanda.destroy + redirect_to demandas_path, notice: 'Demanda excluída com sucesso.' + else + redirect_to demandas_path, alert: erro_de_exclusao + end end private + # Um `before_destroy` que dá `throw :abort` faz #destroy devolver false + # sem popular errors — daí o fallback, para a tela nunca ficar com um + # alerta vazio. + def erro_de_exclusao + @demanda.errors.full_messages.to_sentence.presence || 'Não foi possível excluir a demanda.' + end + def set_demanda @demanda = Demanda.find(params[:id]) end diff --git a/spec/requests/api/v1/demandas_spec.rb b/spec/requests/api/v1/demandas_spec.rb index 384c2cb..bf9cf7d 100644 --- a/spec/requests/api/v1/demandas_spec.rb +++ b/spec/requests/api/v1/demandas_spec.rb @@ -146,5 +146,35 @@ end.to change(Demanda, :count).by(-1) expect(response).to have_http_status(:no_content) end + + # Mesmo caso do spec da tela web: nenhum impeditivo existe hoje, então + # o bloqueio é simulado. O que se verifica é que a API olha o retorno + # de #destroy antes de responder 204. + it 'responde 422 com os erros quando a exclusão é bloqueada' do + bloqueada = Demanda.find(demanda.id) + bloqueada.errors.add(:base, 'Existe um apontamento vinculado a esta demanda.') + allow(Demanda).to receive(:find).and_return(bloqueada) + allow(bloqueada).to receive(:destroy).and_return(false) + + sign_in lider + expect do + delete "/api/v1/demandas/#{demanda.id}", as: :json + end.not_to change(Demanda, :count) + + expect(response).to have_http_status(:unprocessable_content) + expect(response.parsed_body['errors']).to include('Existe um apontamento vinculado a esta demanda.') + end + + it 'devolve uma mensagem genérica quando a exclusão falha sem popular errors' do + bloqueada = Demanda.find(demanda.id) + allow(Demanda).to receive(:find).and_return(bloqueada) + allow(bloqueada).to receive(:destroy).and_return(false) + + sign_in lider + delete "/api/v1/demandas/#{demanda.id}", as: :json + + expect(response).to have_http_status(:unprocessable_content) + expect(response.parsed_body['errors']).to eq(['Não foi possível excluir a demanda.']) + end end end diff --git a/spec/requests/demandas_spec.rb b/spec/requests/demandas_spec.rb index 67ac92b..46feed3 100644 --- a/spec/requests/demandas_spec.rb +++ b/spec/requests/demandas_spec.rb @@ -418,6 +418,43 @@ def results_table(response) delete "/demandas/#{demanda.id}" end.to change(Demanda, :count).by(-1) expect(response).to redirect_to(demandas_path) + follow_redirect! + expect(response.body).to include('Demanda excluída com sucesso.') + end + + # Nenhum impeditivo de exclusão existe hoje (ver o comentário em + # DemandasController#destroy), então o bloqueio é simulado: o que se + # verifica é que a tela olha o retorno de #destroy, e não que ela + # sempre anuncia sucesso. + it 'mostra o erro quando a exclusão é bloqueada, em vez de anunciar sucesso' do + bloqueada = Demanda.find(demanda.id) + bloqueada.errors.add(:base, 'Existe um apontamento vinculado a esta demanda.') + allow(Demanda).to receive(:find).and_return(bloqueada) + allow(bloqueada).to receive(:destroy).and_return(false) + + sign_in lider + expect do + delete "/demandas/#{demanda.id}" + end.not_to change(Demanda, :count) + + expect(response).to redirect_to(demandas_path) + follow_redirect! + expect(response.body).to include('Existe um apontamento vinculado a esta demanda.') + expect(response.body).not_to include('Demanda excluída com sucesso.') + end + + # Um before_destroy com `throw :abort` devolve false sem popular + # errors — sem o fallback, a tela mostraria um alerta vazio. + it 'usa uma mensagem genérica quando a exclusão falha sem popular errors' do + bloqueada = Demanda.find(demanda.id) + allow(Demanda).to receive(:find).and_return(bloqueada) + allow(bloqueada).to receive(:destroy).and_return(false) + + sign_in lider + delete "/demandas/#{demanda.id}" + + follow_redirect! + expect(response.body).to include('Não foi possível excluir a demanda.') end end end