Melhorar legibilidade

[julianalucena]

Seria mais interessante se o nome da chave fosse subject_name.

https://github.com/redu/apps/blob/master/app/models/subject.rb#L21

[embs]

@julianalucena, fiz a modificação neste branch mas notei uma particularidade.

Há as opções de criar a aula com o aplicativo num novo módulo ou num módulo já existente. Acontece que, quando o usuário opta por criar num módulo já existente, a variável subject (que no branch em questão modifiquei para subject_name) assume um valor que é o ID do módulo existente. Assim sendo, talvez fizesse mais sentido adotar o nome de variável mais genérico – subject. Qual tua opinião a respeito?

[julianalucena]

@embs, tu não pode usar dois parâmetros diferentes? Um pra cada, já que se tratam de coisas diferentes.

Inclusive, no controlador, tu poderia deixar de usar o create_subject e diferenciar o caminho pela variável que estiver presente.

[embs]

@julianalucena, essa abordagem entra em conflito com a maneira com que implementamos a action desse controlador. No get_params eu tenho que fazer uma espécie de asserção sobre os parâmetros que devem estar presentes. Assim sendo, caso hajam dois parâmetros distintos para subject (E.G.: subject_name e subject_id) e ambos estejam nessa assertiva, Invalid State sempre será lançado – por conta da ausência de um deles.

Fiquei em dúvida quanto à ideia de que são coisas diferentes. Tanto o ID quanto o nome do Subject podem servir como identificadores do módulo (já que é impossível passar uma entidade para o controlador). Nesse sentido, me parece razoável utilizar um nome genérico (como subject para referenciar o ID ou o nome do módulo), não?

A melhor alternativa que pude pensar foi a de quebrar a ideia da utilização do get_params nesse step_4 – isto é, utilizar params[:subject_name] e params[:subject_id] em vez do get_params para transformar em @subject.

O que achas?

[julianalucena]

Se deixarmos de usar a lógica do get_params, ele não vai levantar o erro caso não tenha a informação do subject.

Eu ainda prefiro chamarmos de algo que não pareça ser uma instância de subject, o que tu acha de subject_param ou subject_info? Algo nesta linha.

[guiocavalcanti]

@embs esse pull request introduziu um bug. Você alterou a implementação mas esqueceu de alterar os testes correspondentes, dá uma olhada no meu último commit.

[embs]

Reabri o issue imaginando que teria de corrigir mas já está corrigido mesmo. Confundi-me com os nomes das variáveis. O nome que muda é o parâmetro que vai da view para o controlador (que passa a ser subject_info). O nome da variável passada do controlador ao modelo (para criação de Subject via API) continua sendo subject_name. Valeu, @guiocavalcanti.