Skip to content

Comments

Models 1#1

Open
pasaunders wants to merge 27 commits intomasterfrom
models-1
Open

Models 1#1
pasaunders wants to merge 27 commits intomasterfrom
models-1

Conversation

@pasaunders
Copy link
Owner

No description provided.

Copy link

@nhuntwalker nhuntwalker left a comment

Choose a reason for hiding this comment

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

Iterate on this.

.travis.yml Outdated
simplegeneric==0.8.1
six==1.10.0
traitlets==4.3.1
wcwidth==0.1.7

Choose a reason for hiding this comment

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

You seem to have mis-named your travis.yml file. This is a requirements file.

README.md Outdated
```

Django will typically serve on port 8000, unless you specify otherwise.
You can access the locally-served site at the address `http://localhost:8000`. No newline at end of file

Choose a reason for hiding this comment

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

....you weren't meant to copy the entire thing wholesale. Make sure it says what you want it to say. What about starting up the postgres database?

blank=True
)
address = models.CharField(max_length=40, null=True, blank=True),
bio = models.TextField(default="")

Choose a reason for hiding this comment

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

or you could have blank/null = True


def __str__(self):
"""Display user data as a string."""
return "User: {}, Camera: {}, Address: {}, Phone number: {} For Hire? {}, Photography style: {}".format(self.user, self.camera_type, self.address, self.phone_number, self.for_hire, self.photography_type)

Choose a reason for hiding this comment

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

You probably want a comma after Phone number: {}


model = User

username = factory.Sequence(lambda n: "The Chosen {}".format(n))

Choose a reason for hiding this comment

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

Again, you don't have to do exactly what I did in class here.

"""Test user has profile attached."""
user = self.users[0]
self.assertTrue(hasattr(user, "profile"))
self.assertIsInstance(user.profile, ImagerProfile)

Choose a reason for hiding this comment

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

You need more tests. Test that the model manager you created works correctly. Test that the attributes that you expect are on the model. Test it inside and out so that there's no mystery as to what the model has on it (in terms of attributes) and what it can do.

'default': {
'ENGINE': 'django.db.backends.postgresql_psycopg2',
'NAME': os.environ['IMAGER_DATABASE'],
# 'USER': os.environ['DATABASE_USER'],

Choose a reason for hiding this comment

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

If one computer needs the user and password, and the other doesn't, then you can just do:

'NAME': os.environ.get('DATABASE_USER', ''),
'PASSWORD': os.environ.get('DATABASE_PASSWORD', '')

@@ -0,0 +1,18 @@
decorator==4.0.11

Choose a reason for hiding this comment

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

This file shouldn't exist in this directory.

def get_queryset(self):
"""Return active users."""
qs = super(ActiveProfileManager, self).get_queryset()
return qs.filter(user__is_active__exact=True)

Choose a reason for hiding this comment

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

Why do you need user__is_active__exact instead of just user__is_active?

'USER': os.environ.get('DATABASE_USER', ''),
'PASSWORD': os.environ.get('DATABASE_PASSWORD', ''),
# 'HOST': '127.0.0.1',
# 'PORT': '5432',

Choose a reason for hiding this comment

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

Why are these commented?

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