redhat-beyond / JobSeeker

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

🐛 Personal Profile: Leftover Images After Running Pytest #68

Open taljacob2 opened 2 years ago

taljacob2 commented 2 years ago

Description

Hi, In the Personal-Profile app, I found out that after running:

pipenv run python pytest -v

the test pass, but create leftover images. We need pytest to delete them automatically.

See the screenshot (commit 5011071733e6e754ae1727eaaeffcfedaaf8af7c):

image

Also in PR #65 (commit dcef35d276e1cc84c5581030a102dc4762382816):

image

ShirleyFichman commented 2 years ago

Hi, it seems that we have 2 problems in need of fixing here:

  1. Deleting the files created from the tests, as can be seen, named "test_ resume" and "test_image".
  2. Not creating multiple files from the data migrations (we have 4 users and whenever the migrations run they create more files each, although already exist).
image

I've tried many ways already: trying to delete them manually at the end of a test, using "get_or_create" instead of "create" in the data migrations, and also many different changes with no success. Will continue to look further into it.

taljacob2 commented 2 years ago

(The code I used here is: 262803a. It is the updated version in the PR of #65 right now)

Walkthrough Of What I Have Done To View The Problem

Clean The Database

Steps I have made to setup a fresh start for the database:

  • I have run vagrant up.
  • I have killed Django server, with:
    vagrant ssh
    cd /vagrant
    killall -9 python
    exit
  • I have deleted the database (db.sqlite3 file).

Recreate The Problem

This has created the image files in the backend (that their name is appended with a generated ID, as expected), and inserted their path string to the database.

Let's view it in the database through the shell:

For example, let's focus John Doe.

As you can see, this works fine. The profile_pic and resume return the path to the saved images we saw in the shell.

Now let's check this on admin view:

It seems that the images in John Doe's profile are correctly linked to the images created in the backend:

PROBLEM FOUND

Let's try to delete John Doe's profile, and see if the images get deleted from the backend too.

PROBLEM

The images are still in the backend, though we have deleted John Doe's personal_profile :

Conclusion

So this is why although pytest's tests delete the profile_profiles they create, the images that were created still don't get deleted, and remain in the backend.

Suggestion (@ShirleyFichman tell me your thoughts about this)

Create our own implementation for PersonalProfile.delete that:

  1. Deletes its created images in the backend:
    • Deletes the image that is in the path of: personal_profile.profile_pic
    • Deletes the image that is in the path of: personal_profile.resume

      We can try this StackOverflow Answer for deleting Pillow images.

  2. And deletes the personal_profile's record from the database (as the ORM does).
taljacob2 commented 2 years ago

Suggested Implementation for personal_profile.delete

Override personal_profile.delete (thanks to this StackOverflow Answer), by navigating to personal_profile/models.py, and in the PersonalProfile class, add the following method:

import os
from app.settings import MEDIA_ROOT

class PersonalProfile(models.Model):

    def delete(self):

        # Get the media root path, and remove the first `/vagrant` folder.
        media_root_path = os.path.join(*(MEDIA_ROOT.split(os.path.sep)[2:]))

        file_paths_to_delete = [media_root_path + "/" + self.profile_pic.name,
                                media_root_path + "/" + self.resume.name]

        # Filter list.
        file_paths_to_delete = [i for i in file_paths_to_delete if
                                i is not None]

        # Delete files from the backend.
        if file_paths_to_delete:
            for file_path in file_paths_to_delete:
                print(f'deleting {file_path}')
                os.remove(file_path, dir_fd=None)

        # Call Django's ORM to delete this record.
        super(PersonalProfile, self).delete()
ShirleyFichman commented 2 years ago

Suggested Implementation for personal_profile.delete

Override personal_profile.delete (thanks to this StackOverflow Answer), by navigating to personal_profile/models.py, and in the PersonalProfile class, add the following method:

import os
from app.settings import MEDIA_ROOT

class PersonalProfile(models.Model):

    def delete(self):

        # Get the media root path, and remove the first `/vagrant` folder.
        media_root_path = os.path.join(*(MEDIA_ROOT.split(os.path.sep)[2:]))

        file_paths_to_delete = [media_root_path + "/" + self.profile_pic.name,
                                media_root_path + "/" + self.resume.name]

        # Filter list.
        file_paths_to_delete = [i for i in file_paths_to_delete if
                                i is not None]

        # Delete files from the backend.
        if file_paths_to_delete:
            for file_path in file_paths_to_delete:
                print(f'deleting {file_path}')
                os.remove(file_path, dir_fd=None)

        # Call Django's ORM to delete this record.
        super(PersonalProfile, self).delete()

@taljacob2 Thank you so much for your kind help! Unfortunately, this still doesn't work, I'm trying to go in that direction of trying to specifically delete the files from the db.

ShirleyFichman commented 2 years ago

I used https://stackoverflow.com/questions/16041232/django-delete-filefield to fix part of the bug- specifically, delete the files that were created for the tests! The rest of the "doubled" files are created as part of the data migrations, I need to find a solution for that as well- but it is not connected to the tests.