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

Feature/bben 144 creating test models #246

Closed
wants to merge 17 commits into from

Conversation

cristophersfr
Copy link

This PR refers to the following issues described at JIRA:

  • BBEN 144
  • BBEN 145
  • BBEN 146

In summary, these modifications create a separated model for handling the Test functionality that is being built. Within this implementation, we also consider that the model for Test, must not create new intents, entities, and labels without considering the existing ones.

@coveralls
Copy link

coveralls commented Mar 26, 2019

Pull Request Test Coverage Report for Build 1274

  • 491 of 502 (97.81%) changed or added relevant lines in 8 files are covered.
  • No unchanged relevant lines lost coverage.
  • Overall coverage increased (+0.2%) to 96.154%

Changes Missing Coverage Covered Lines Changed/Added Lines %
bothub/api/v2/validation/validators.py 24 26 92.31%
bothub/common/models.py 31 34 91.18%
bothub/api/v2/validation/filters.py 29 35 82.86%
Totals Coverage Status
Change from base Build 1265: 0.2%
Covered Lines: 3950
Relevant Lines: 4108

💛 - Coveralls

from bothub.api.v1.fields import LabelValueField

from bothub.api.v1.validators import CanContributeInRepositoryValidator
from bothub.api.v1.validators import EntityNotEqualLabelValidator
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

A API V1 em algum momento será obsoleta/descontinuada, ficando a V2 como padrão, então não faz sentido utilizar na V2 metodos, classes ou demais coisas vinda da V1.

Copy link
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Qual seria a melhor abordagem? Copiar essas implementações para a V2 ?

return obj.entity.label.value


class NewRepositoryValidationEntitySerializer(serializers.ModelSerializer):
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

No meu ponto de vista não é necessário ter um serializer apenas para "create" sendo que para "retrieve" tem outro que no fim é referente ao mesmo model "RepositoryValidationEntity"

)})


class RepositoryValidationWithIntentOrEntityValidator(object):
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Renomear essa classe para algo mais curto e legivel, não utilizar "or".

from .filters import ValidationFilter


class NewValidationViewSet(
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Essa ação de "NewValidation" não é necessária e pode está junto com a "ListValidationViewSet" e seu respectivo mixins.

migrations.DeleteModel(
name='RepositoryTranslatedValidationEntity',
),
]
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Como é uma feature nova não tem logica mandar 5, 6 migrations fazendo criação, alteração, remoção de campos... remove esses arquivos e gera um novo já com a representação final dos modelos.

@VictorMeneghini VictorMeneghini deleted the feature/BBEN-144-creating-test-models branch July 8, 2019 12:26
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
enhancement New feature or request
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

4 participants