Skip to content
This repository has been archived by the owner on Apr 14, 2023. It is now read-only.

vacancy: Podem ser criadas duas call-cargo-região-restrição iguais. Isso é certo? #17

Open
annecchini opened this issue May 9, 2020 · 3 comments

Comments

@annecchini
Copy link
Contributor

O modelo Vacany permite a criação de duas entradas "iguais" no banco:

call-cargo-região-restrição deveria ser único?

Porque isso pode confundir um pouco o operador do sistema.

Ex: Posso para a mesma call criar duas vezes uma oferta de vaga de aluno para vitória, sem restrição.

@edo9k
Copy link
Collaborator

edo9k commented May 11, 2020

Sim, talvez seja interessante além de validar validators/ também inserir a regra no banco, colocando um unique nos campos assignment_id, restriction_id, call_id, region_id.

To tentando pensar em alguma implicação disso, algum caso que vamos querer que seja possível duplicar, mas não to conseguindo pensar em nenhum.

@annecchini
Copy link
Contributor Author

annecchini commented May 15, 2020

01 - Você encontrará instruções para criar uma chave única em um modelo que usa paranoid nesse arquivo:
a) Nos dois lugares onde tem 't.rollback()' use 'await t.rollback()' essa é uma correção a fazer em migrations desse tipo.
b) Essa operação funcionou sem 'await t.commit()', mas tive problemas sem isso em uma seed.

https://github.com/SEAD-UFES/publications/blob/development/db/migrations/20200409133150-rebuild-UserRole.js

02 - Você encontrará um esqueleto para criar a validação da chave única seguindo mais ou menos um padrão nesse arquivo/linha:
a) Esse é meu projeto pessoal não cheguei a fazer uma validação desse tipo no backend.
b) Depois de ler e entender a função, verifique como ela foi usada em 'validateBody()'.

https://github.com/annecchini/my-sps-server/blob/master/app/validation/process.js#L72

03 - Estou pedindo para que seja feito:
a) Decida se vai ou não colocar essa validação a nível de banco.
b) Se decidiu que sim, faça a migration.
c) Crie a validação em app/validator/vacancy
d) timestamps:true e paranoid:true no model
e) Inserir id no model (com defaultValue, não precisa de vacancy,beforeCreate())

id: {
        type: DataTypes.UUID,
        defaultValue: DataTypes.UUIDV4,
        primaryKey: true,
        allowNull: false
}

Se você tiver dúvidas sobre como proceder, fala comigo. Vou tirar suas dúvidas.

@annecchini
Copy link
Contributor Author

annecchini commented May 21, 2020

Olá.
A validação de unique no validator está funcionando, mas vou sugerir uma mudança e explicar o porque:

Mudança 01:

01 - Inserir o objeto "item" na função de validação. Você deve ter notado que as funções de validação tem um objeto "item" e "mode" dentro delas. Não está sendo muito usado para o caso de vacancy.

02 - Para que serve "item": item é uma instancia do objeto que eu quero atualizar, no caso do create ele não existe. Com esse objeto na função, é possível fazer atualização parcial do objeto (mandar apenas region_id no PUT por exemplo) mas isso vem com o preço de que, para cada item que entrar na função deve-se decidir qual dos dois valores será usado (body.restriction_id ou item.restriction_id por exemplo)

restriction_id = body.restriction_id || body.restriction_id === null ? body.restriction_id : item.restriction_id

03 - As outras validações (qtd, call_id, etc...) partem do principio de que eu posso (ou não) enviar os campos, isso é feito usando 2 estratégias:

//value is necessary (obrigatorio apenas no create)
 if (typeof value === 'undefined' && mode === 'create') {
   return 'Este campo é necessário.'
 }

 //value is valid (vou validar apenas se o value foi enviado)
 if (typeof value !== 'undefined' && (value === null || value === '')) {
   return 'Este campo é requerido.'
 }

04 - Então seria legal se a validação de duplicata também levasse isso em consideração. (P/S: considerando que essas validações estão sendo usadas apenas em create por enquanto e que estão faltando um serie de mecanismos para tornar esse método perfeito em vacancy, se você for testar em vacancy (atualização parcial) vai dar merda. Se você quiser detalhes dessa técnica funcionando. sugiro dar uma olha da no branch (calendar_crud)

Mudança 02:

Essa validação vai criar um problema em update, já que não existe a exceção para ele mesmo no update. (Por isso inserir "mode" também se faz necessário)

Mudança 03:

05 - Dito isso, essa validação deveria apenas ser feita seu eu não tiver erro nas validações anteriores dos campos envolvidos. Acredito que nesse caso não vai dar problema, mas se fosse algo que requer uma operação com os valores (valueA > valueB) e eu tivesse valores inválidos, a validação ficaria prejudicada.
CORREÇÃO: Quando você fizer a Mudança 01, vai dar problema sim, por que como a existência de um valor como call_id não está sendo considerada no validateDuplicate, ele vai tentar usar o valor de "item" que nesse caso não existe. item.call_id (Erro: item undefined)

Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

No branches or pull requests

2 participants