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

✨ Adding importer models - #111

Open
evict wants to merge 23 commits into
developfrom
vincent-dev
Open

✨ Adding importer models#111
evict wants to merge 23 commits into
developfrom
vincent-dev

Conversation

@evict

@evict evict commented Feb 8, 2019

Copy link
Copy Markdown
Member
  • 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
Copy Markdown

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
Copy Markdown

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
Copy Markdown

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
Copy Markdown
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
Copy Markdown

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
Copy Markdown
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)

Comment thread openbook_importer/views.py Outdated
Comment thread openbook_importer/views.py Outdated
Comment thread openbook_auth/models.py
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
Copy Markdown
Member

Choose a reason for hiding this comment

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

cover

@qlty-cloud-legacy

Copy link
Copy Markdown

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

uiboy commented Feb 14, 2019

Copy link
Copy Markdown
Member

Shouldnt we be merging into develop ?

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

friend = ImportedFriend.objects.filter(friend_hash=friend_hash)

Copy link
Copy Markdown
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
Copy Markdown
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
Copy Markdown
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
Copy Markdown
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 qlty-cloud-legacy 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