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

feat: Add form and question models #25

Merged
merged 3 commits into from
Jun 28, 2020

Conversation

bismitaguha
Copy link
Contributor

Description

  • Added Form and Question models
  • Added AbstractTimestamp model
  • Added related Field models

Fixes #17

Type of Change:

  • Code
  • Documentation

Code/Quality Assurance Only

  • New feature (non-breaking change which adds functionality pre-approved by mentors)

How Has This Been Tested?

The models are created and migrations run:
Screenshot from 2020-06-20 21-04-18

Checklist:

  • My PR follows the style guidelines of this project
  • I have performed a self-review of my own code or materials
  • I have commented on my code or provided relevant documentation, particularly in hard-to-understand areas
  • Any dependent changes have been merged

Code/Quality Assurance Only

  • My changes generate no new warnings
  • New and existing unit tests pass locally with my changes
  • Any dependent changes have been published in downstream modules

Copy link
Contributor

@Monal5031 Monal5031 left a comment

Choose a reason for hiding this comment

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

file line ending issues are in every file, make sure to check your IDE settings

osp/admin.py Outdated
@@ -3,5 +3,13 @@
from osp.models import *
Copy link
Contributor

Choose a reason for hiding this comment

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

No! Please don't. Never do this

from osp.models.question import Question
from osp.models.abstract_timestamp import AbstractTimestamp
from osp.models.field import *
Copy link
Contributor

Choose a reason for hiding this comment

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

Same here

)

class Meta:

Copy link
Contributor

Choose a reason for hiding this comment

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

extra line, please remove

Abstract model for Created Time and Updated Time fields
"""

created_on = models.DateTimeField(
Copy link
Contributor

Choose a reason for hiding this comment

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

single param, same line should suffice


from osp.models.abstract_timestamp import AbstractTimestamp

SA = 'char'
Copy link
Contributor

Choose a reason for hiding this comment

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

Create a separate directory for utilities, This new dir should contain all the abstract or generic models, functions, choices, views and manager.

osp/models/question.py Show resolved Hide resolved

def __str__(self):

return f'{self.name}'
Copy link
Contributor

Choose a reason for hiding this comment

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

F formatting for a single param doesn't makes sense

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Without f the display is {self.name}. I feel it is needed!

description = models.TextField(
blank=True
)
created_on = models.DateTimeField(
Copy link
Contributor

Choose a reason for hiding this comment

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

You are already inheriting AbstractTimestamp. no need to add these 2 fields again

osp/models/field.py Show resolved Hide resolved
osp/models/question.py Show resolved Hide resolved
Copy link

@sidvenu sidvenu left a comment

Choose a reason for hiding this comment

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

Please make the changes suggested by @Monal5031. I don't see any other changes.

@Monal5031 Monal5031 merged commit 07f1d5f into anitab-org:develop Jun 28, 2020
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.

Create back-end model classes for forms
3 participants