Skip to content
This repository has been archived by the owner on Feb 4, 2022. It is now read-only.

✨ Adding importer models #111

Open
wants to merge 23 commits into
base: develop
Choose a base branch
from
Open

✨ Adding importer models #111

wants to merge 23 commits into from

Conversation

evict
Copy link
Member

@evict evict commented Feb 8, 2019

  • Created importer related models and tests;
  • Added view for viewing imports and deletion;
  • Commit to openbook_auth.models User model for deleting imports.

return True

else:
return False
Copy link

Choose a reason for hiding this comment

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

Avoid too many return statements within this function.

if friend.exists():
friend = friend[0]

if friend.user1_id == user.pk and friend.user2_id:
Copy link

Choose a reason for hiding this comment

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

Consider simplifying this complex logical expression.

friend.user2 = user
friend.save()

return True
Copy link

Choose a reason for hiding this comment

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

Avoid too many return statements within this function.

Copy link
Member

Choose a reason for hiding this comment

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

I assume the find_friend function here is looking for diff combinations where the two were already friends on facebook and if yes it saves the relationship.. so do we really need to return anything here? Also its not clear who we are finding friends for.. the user looking through the hash, or the hash looking into the user....
maybe consider renaming to find_friend_for_user ..

friend.user1 = user
friend.save()

return True
Copy link

Choose a reason for hiding this comment

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

Avoid too many return statements within this function.

user = make_user()
headers = make_authentication_headers_for_user(user)

with open('openbook_importer/tests/facebook-jayjay6.zip',
Copy link
Member

Choose a reason for hiding this comment

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

Use a fake import

def make_import():
    return mixer.blend(Import)

To create multiple

users = mixer.cycle(amount).blend(User)

openbook_importer/views.py Outdated Show resolved Hide resolved
openbook_importer/views.py Outdated Show resolved Hide resolved
@@ -1244,7 +1260,8 @@ class UserProfile(models.Model):
location = models.CharField(_('location'), max_length=settings.PROFILE_LOCATION_MAX_LENGTH, blank=False, null=True)
user = models.OneToOneField(settings.AUTH_USER_MODEL, on_delete=models.CASCADE, related_name='profile')
birth_date = models.DateField(_('birth date'), null=False, blank=False)
avatar = models.ImageField(_('avatar'), blank=False, null=True)
avatar = ProcessedImageField(verbose_name=_('avatar'), processors=[ResizeToFill(500, 500)] ,blank=False, null=True, format='JPEG',
options={'quality': 75}, )
cover = models.ImageField(_('cover'), blank=False, null=True)
Copy link
Member

Choose a reason for hiding this comment

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

cover

@codeclimate
Copy link

codeclimate bot commented Feb 9, 2019

Code Climate has analyzed commit 2f97590 and detected 9 issues on this pull request.

Here's the issue category breakdown:

Category Count
Complexity 9

The test coverage on the diff in this pull request is 86.8% (50% is the threshold).

This pull request will bring the total coverage in the repository to 84.8% (1.5% change).

View more on Code Climate.

@uiboy
Copy link
Member

uiboy commented Feb 14, 2019

Shouldnt we be merging into develop ?

@classmethod
def find_friend(cls, friend_hash, user):

friend = ImportedFriend.objects.filter(friend_hash=friend_hash)
Copy link
Member

Choose a reason for hiding this comment

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

if there's going to be multiple friends, call it friends... if not you can use ImportedFriend.objects.get to get just one then you dont have to use friend[0] .. Also the get method raises a ImportedFriend.DoesNotExist error if nothing is found,

friend.user2 = user
friend.save()

return True
Copy link
Member

Choose a reason for hiding this comment

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

I assume the find_friend function here is looking for diff combinations where the two were already friends on facebook and if yes it saves the relationship.. so do we really need to return anything here? Also its not clear who we are finding friends for.. the user looking through the hash, or the hash looking into the user....
maybe consider renaming to find_friend_for_user ..

fields = (
'uuid',
'created',
'posts'
Copy link
Member

Choose a reason for hiding this comment

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

This serializer will return posts as an array of Ids if there is a foreignKey relationship... is that your intention?

If you want to return the full post you need to add a serializer for posts. Something like

class ImportSerializer(serializers.ModelSerializer):
   posts = ImportedPostSerializer(many=True)
    class Meta:
        model = Import
        fields = (
            'uuid',
            'created',
            'posts'

Copy link
Member Author

Choose a reason for hiding this comment

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

We do not want to return the full post, only the number. It is a custom property in the model.

@evict evict changed the base branch from master to develop February 16, 2019 14:36
@OkunaOrg OkunaOrg deleted a comment from codeclimate bot May 12, 2019
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants