search

psf / python-in-edu

website for educational python resources
http://education.python.org/
MIT License
41 stars 28 forks source link

Issue 25 attribution change #48

Open brandonrosenbloom opened 3 years ago

brandonrosenbloom commented 3 years ago

SEVERAL CHANGES: 1) Added .idea files to gitignore to help keep the repo clean 2) Added several models to the Resources app in order to help better organize the data while also providing future adjustability. 3) Created a basic author form so new authors could be created while also adding the necessary admin settings. STILL NOT DONE.

brandonrosenbloom commented 3 years ago

@sijanonly I have now added the data migration file. You should be able to pull the latest version of this request down and run migrate to see the data appear.

sijanonly commented 3 years ago

@sijanonly I have now added the data migration file. You should be able to pull the latest version of this request down and run migrate to see the data appear.

@brandonrosenbloom I am still getting this issue while running migration.

can you please have a look?

mig_issue

brandonrosenbloom commented 3 years ago

@sijanonly Try now. I've updated the get_default_value method to return a none if there are no values available.

sijanonly commented 3 years ago

@sijanonly Try now. I've updated the get_default_value method to return a none if there are no values available.

@brandonrosenbloom , the changes didn't work. Actually, the statement ResourceStatus.objects.get(sequence=1).id will raise error since it won't find the ResourceStatus object. I changed it to following and it works.

def get_initial_status():
    try:
        initial_status = ResourceStatus.objects.get(sequence=1).id
    except ResourceStatus.DoesNotExist:
        initial_status = None
    return initial_status

Also, there is one issue I am facing while creating user. I tried to create superuser python manage.py createsuperuser and Profile model has populations field, which is NOT NULL. And it raises django.db.utils.IntegrityError: NOT NULL constraint failed: resources_profile.populations_id

I think it's better to set null=True for populations field in Profile model.

brandonrosenbloom commented 3 years ago

@sijanonly I've applied the change to the get_initial_status method as suggested. @meg-ray Do we want to guarantee that all profiles have a population entered?

sijanonly commented 3 years ago

@sijanonly I've applied the change to the get_initial_status method as suggested. @meg-ray Do we want to guarantee that all profiles have a population entered?

@brandonrosenbloom I just found that there is a separate ProfileUpdateView where user can update populations,.. information. It seems like we can put None for default ?

brandonrosenbloom commented 3 years ago

@sijanonly The only issue I see with setting the population attribute to None on the model is really how it may impact the business process by which users are created. @meg-ray if we don't expect new users to always be created with a population value, then setting the value to no longer be required makes sense. However, if we expect all new users to have a population to be identified when they are first created, then we should probably leave the field as required. Thoughts?