redhat-beyond / JobSeeker

https://github.com/redhat-beyond/JobSeeker
MIT License
2 stars 5 forks source link

🚀 Feed: initializing app and adding data models #27

Closed paOmer closed 2 years ago

paOmer commented 2 years ago

This PR is built upon issue #26 Setting up the feed app and adding data models.

Close #26

paOmer commented 2 years ago

I would advise not to focus the GUI, for now concentrate the model Please remove UI parts

@Yarboa done

Yarboa commented 2 years ago

./feed/models/init.py:1:1: F401 '.post.Post' imported but unused ./feed/models/init.py:1:1: F401 '.post.PostManager' imported but unused ./feed/models/init.py:2:1: F401 '.comment.Comment' imported but unused Error: Process completed with exit code 1.

Please fix flake8 errors, no review on failed actions

DeanBiton commented 2 years ago

Tests look solid, I also focused on the creation and deletion of data. Notice that you can also use the @pytest.mark.django_db() mark above a class and it will be relevant for the methods inside it.

paOmer commented 2 years ago

Tests look solid, I also focused on the creation and deletion of data. Notice that you can also use the @pytest.mark.django_db() mark above a class and it will be relevant for the methods inside it.

Cool, Thanks for reviewing :)

paOmer commented 2 years ago

I've populated the DB and added tests to the data models. I needed to populated the DB with preferences which made a job_board test to fail so I changed the deletion tests to delete the specific fixture models and not delete by filters.

Im not sure if the changes I made to the job board are legitimate, so please review it @DeanBiton

taljacob2 commented 2 years ago

@paOmer When I ran vagrant up, I got at the bottom of the output:

    default:   Your models in app(s): 'feed' have changes that are not yet reflected in a migration, and so won't be applied.
    default:   Run 'manage.py makemigrations' to make new migrations, and then re-run 'manage.py migrate' to apply them.
    default: + setsid pipenv run python manage.py runserver 0.0.0.0:8000

So I ran:

pipenv run manage.py makemigrations feed

and a new file called feed/migrations/0012_alter_post_prefernces.py was created, according to the output:

Migrations for 'feed':
  feed/migrations/0012_alter_post_prefernces.py
    - Alter field prefernces on post

and the content of the feed/migrations/0012_alter_post_prefernces.py file is:

# Generated by Django 4.0.3 on 2022-03-30 06:32

from django.db import migrations, models
import django.db.models.deletion

class Migration(migrations.Migration):

    dependencies = [
        ('job_board', '0004_test_data_years_of_experiences'),
        ('feed', '0011_comments'),
    ]

    operations = [
        migrations.AlterField(
            model_name='post',
            name='prefernces',
            field=models.ForeignKey(null=True, on_delete=django.db.models.deletion.SET_NULL, to='job_board.preference'),
        ),
    ]

Is this alright? Do you know this?

paOmer commented 2 years ago

@paOmer When I ran vagrant up, I got at the bottom of the output:

    default:   Your models in app(s): 'feed' have changes that are not yet reflected in a migration, and so won't be applied.
    default:   Run 'manage.py makemigrations' to make new migrations, and then re-run 'manage.py migrate' to apply them.
    default: + setsid pipenv run python manage.py runserver 0.0.0.0:8000

So I ran:

pipenv run manage.py makemigrations feed

and a new file called feed/migrations/0012_alter_post_prefernces.py was created, according to the output:

Migrations for 'feed':
  feed/migrations/0012_alter_post_prefernces.py
    - Alter field prefernces on post

and the content of the feed/migrations/0012_alter_post_prefernces.py file is:

# Generated by Django 4.0.3 on 2022-03-30 06:32

from django.db import migrations, models
import django.db.models.deletion

class Migration(migrations.Migration):

    dependencies = [
        ('job_board', '0004_test_data_years_of_experiences'),
        ('feed', '0011_comments'),
    ]

    operations = [
        migrations.AlterField(
            model_name='post',
            name='prefernces',
            field=models.ForeignKey(null=True, on_delete=django.db.models.deletion.SET_NULL, to='job_board.preference'),
        ),
    ]

Is this alright? Do you know this?

@taljacob2 thanks for letting me know.

That weird, Iv'e destroyed my VM and deleted the db file, the ran vagrant up but didn't got the output that you've got. Anyway I ran pipenv run python manage.py makemigrations and also got the same migration file as you did. I also ran pipenv run python manage.py migrate and everything seems to work find before and after this migration, I mean that I can see that the post model has the preference model linked to it via the admin panel before and after the migration.

I guess everything seems fine, and I'm wondering if this needs to be commited to repo, what are your thoughts about it?

taljacob2 commented 2 years ago

@paOmer When I ran vagrant up, I got at the bottom of the output:

    default:   Your models in app(s): 'feed' have changes that are not yet reflected in a migration, and so won't be applied.
    default:   Run 'manage.py makemigrations' to make new migrations, and then re-run 'manage.py migrate' to apply them.
    default: + setsid pipenv run python manage.py runserver 0.0.0.0:8000

So I ran:

pipenv run manage.py makemigrations feed

and a new file called feed/migrations/0012_alter_post_prefernces.py was created, according to the output:

Migrations for 'feed':
  feed/migrations/0012_alter_post_prefernces.py
    - Alter field prefernces on post

and the content of the feed/migrations/0012_alter_post_prefernces.py file is:

# Generated by Django 4.0.3 on 2022-03-30 06:32

from django.db import migrations, models
import django.db.models.deletion

class Migration(migrations.Migration):

    dependencies = [
        ('job_board', '0004_test_data_years_of_experiences'),
        ('feed', '0011_comments'),
    ]

    operations = [
        migrations.AlterField(
            model_name='post',
            name='prefernces',
            field=models.ForeignKey(null=True, on_delete=django.db.models.deletion.SET_NULL, to='job_board.preference'),
        ),
    ]

Is this alright? Do you know this?

@taljacob2 thanks for letting me know.

That weird, Iv'e destroyed my VM and deleted the db file, the ran vagrant up but didn't got the output that you've got. Anyway I ran pipenv run python manage.py makemigrations and also got the same migration file as you did. I also ran pipenv run python manage.py migrate and everything seems to work find before and after this migration, I mean that I can see that the post model has the preference model linked to it via the admin panel before and after the migration.

I guess everything seems fine, and I'm wondering if this needs to be commited to repo, what are your thoughts about it?

I guess eventually we will add this file anyway because Django wants us to. Although I am not sure this file does anything essential. It is possible to decide about this in the future, and leave it like this for now.