Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Revisando as dependências do projeto #57

Merged
merged 8 commits into from
Oct 11, 2018
Merged

Revisando as dependências do projeto #57

merged 8 commits into from
Oct 11, 2018

Conversation

klauskpm
Copy link
Contributor

@klauskpm klauskpm commented Oct 5, 2018

Issue #56

Com o comando npm audit --fix foram corrigidas mais de 200 vulnerabilidades, e atualizei cuidadosamente alguns pacotes (mocha, chai, chai-http, nyc, coverals) que tinham breaking changes, mas, sobraram dois pacotes e algumas correções manuais.

screen shot 2018-10-04 at 22 59 10

Os pacotes que precisam ser atualizados são:
npm install --save-dev [email protected] (3 low)
npm install [email protected] (2 moderate, 2 low)

Não atualizei o eslint, pois também precisei atualizar o standard, e com isso gerou erro na hora de executar o npm run eslint.
Não atualizei o express-validation porque não consigo fazer o projeto rodar. Não teria como garantir se está tudo correto.

@sourcelevel-bot
Copy link

Hello, @klauskpm! This is your first Pull Request that will be reviewed by Ebert, an automatic Code Review service. It will leave comments on this diff with potential issues and style violations found in the code as you push new commits. You can also see all the issues found on this Pull Request on its review page. Please check our documentation for more information.

@coveralls
Copy link

coveralls commented Oct 5, 2018

Coverage Status

Coverage remained the same at 88.38% when pulling 5a04433 on klauskpm:audit into 9545d42 on frontendbr:master.

@angeliski
Copy link
Contributor

Nossa, maravilhoso @klauskpm !
Você pode realizar as duas atualizações restantes?
No caso do eslint, se você executar o comando npm run eslint:fix o problema deveria ser resolvido, se não funcionar você pode comitar que eu verifico o problema.

Quanto a rodar o projeto, você teve alguma dificuldade? Algum erro? Se você puder esclarecer podemos tentar melhorar o README ou outras configurações.

Quando você atualizar o express-validation os testes deveriam pegar qualquer possível erro, mas fique tranquilo que eu rodo um teste local para verificar se o funcionamento se mantém.

No mais, muito obrigado pelo trabalho!

@klauskpm
Copy link
Contributor Author

klauskpm commented Oct 5, 2018

Sem problemas @angeliski!

O erro do eslint foi tanto do padrão, quanto um erro estourando pela execução. Por isso voltei a atualização dele, mas, posso investigar mais. Também vou fazer a atualização do express-validation e rodar os testes. Devo fazer isso mais tarde.

Vi que tem uma issue de README. Posso colocar lá os passos que tomei pra tentar rodar o projeto e os erros que tem acontecido.

@angeliski
Copy link
Contributor

Seria de muita ajuda se vc pudesse colocar os comentários na issue #25 Assim a gente pode melhorar o readme já

@klauskpm
Copy link
Contributor Author

klauskpm commented Oct 6, 2018

Os pacotes foram atualizados e os erros foram corrigidos!

@angeliski
Copy link
Contributor

Valeu @klauskpm ! Eu só devo conseguir testar amanhã, mas valeu pela força!

@angeliski
Copy link
Contributor

Eu testei aqui e ficou show @klauskpm !

Será que você pode só adicionar um teste para cobrir a validação (caso seja necessário atualizar de novo)?

it('should return error when not have title', (done) => {
      const event = {
        'shortDescription': 'Descrição marota',
        'status': 'pending',
        'link': 'https://braziljs.org/conf/',
        'price': 1.5,
        'image': 'https://braziljs.org/wp-content/themes/braziljs/assets/img/logos/braziljs-00508dcfc4.svg',
        '__v': 0,
        'location': {
          'city': 'Fortaleza',
          'state': 'CE',
          'address': 'Faculdade Sete de Setembro',
          'locationUrl': 'https://www.google.com.br/maps'
        },
        'date': {
          'day': 1,
          'month': 'Setembro',
          'year': 2017
        }
      }

      const erroBody = {
        'status': false,
        'data': {
          'errors': {
            'title': {
              'message': 'Path `title` is required.',
              'name': 'ValidatorError',
              'properties': {
                'type': 'required',
                'message': 'Path `{PATH}` is required.',
                'path': 'title'
              },
              'kind': 'required',
              'path': 'title',
              '$isValidatorError': true
            }
          },
          '_message': 'Event validation failed',
          'message': 'Event validation failed: title: Path `title` is required.',
          'name': 'ValidationError'
        }
      }

      request.post('/')
        .send(event)
        .set('authorization', `Bearer ${token}`)
        .expect('Content-Type', /json/)
        .expect(500, erroBody)
        .end(TestUtil.endTest.bind(null, done))
    })
  })

Você pode adicionar ele no event.integration.spec.js, depois do teste should return error when not have price

@klauskpm
Copy link
Contributor Author

klauskpm commented Oct 9, 2018

Com toda certeza!

@klauskpm
Copy link
Contributor Author

Mal a demora @angeliski, mas está feito!

@angeliski angeliski merged commit a253188 into frontendbr:master Oct 11, 2018
@angeliski
Copy link
Contributor

Tá na master! Valeu pela contribuição cara!

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

Successfully merging this pull request may close these issues.

3 participants