python-in-edu icon indicating copy to clipboard operation
python-in-edu copied to clipboard

Issue 25 attribution change

Open brandonrosenbloom opened this issue 3 years ago • 7 comments

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 avatar May 17 '21 14:05 brandonrosenbloom

@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 avatar Jun 08 '21 18:06 brandonrosenbloom

@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

sijanonly avatar Jun 08 '21 19:06 sijanonly

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

brandonrosenbloom avatar Jun 08 '21 21:06 brandonrosenbloom

@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.

sijanonly avatar Jun 09 '21 07:06 sijanonly

@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 avatar Jun 09 '21 20:06 brandonrosenbloom

@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 ?

sijanonly avatar Jun 13 '21 17:06 sijanonly

@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?

brandonrosenbloom avatar Jun 14 '21 17:06 brandonrosenbloom