Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

162 Move tag stuff from SE site to catalog module #163

Merged
merged 5 commits into from
Aug 25, 2018
Merged

Conversation

duker33
Copy link
Collaborator

@duker33 duker33 commented Aug 23, 2018

@duker33 duker33 self-assigned this Aug 23, 2018
Copy link
Contributor

@ArtemijRodionov ArtemijRodionov left a comment

Choose a reason for hiding this comment

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

One moment with a test class

default=0, blank=True, db_index=True, verbose_name=_('position'),
)

slug = models.SlugField(default='')
Copy link
Contributor

@ArtemijRodionov ArtemijRodionov Aug 23, 2018

Choose a reason for hiding this comment

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

Let's add unique constraint to it. It will help to ensure consistency of db

unidecode(self.name.replace('.', '-').replace('+', '-'))
)
doubled_tag_qs = self.__class__.objects.filter(slug=self.slug)
if doubled_tag_qs:
Copy link
Contributor

Choose a reason for hiding this comment

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

doubled_tag_qs.exists() may be better for this

))


def serialize_tags(
Copy link
Contributor

Choose a reason for hiding this comment

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

Seems Tag.objects.serialize_to_url(...) looks better and maybe more convenient, so let's create todo for it

@@ -38,3 +41,27 @@ def get_absolute_url(self):
@property
def image(self):
return 'no-image-right-now'


class TagTest(TestCase):
Copy link
Contributor

Choose a reason for hiding this comment

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

This module serves to implement mock models, but not for tests, so let's remove this class

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

@artemiy312 , i meant TagModel module from SE.
We'll move it from SE to this place in with pdd issue defined below.
https://github.com/fidals/shopelectro/blob/master/shopelectro/tests/tests_models.py#L30

I'll rename it

Copy link
Contributor

@ArtemijRodionov ArtemijRodionov left a comment

Choose a reason for hiding this comment

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

One moment with a test class

))


# @todo #162:15m Move serialize_tags TagQuerySet's method
Copy link
Contributor

Choose a reason for hiding this comment

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

Move serialize_tags to ...

@duker33 duker33 merged commit 512cfa8 into master Aug 25, 2018
@duker33 duker33 deleted the 162_take_tag_stuff branch August 25, 2018 08:05
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants