Ircam-WAM / mezzanine-organization

Organization module for the Mezzo CMS
GNU Affero General Public License v3.0
2 stars 0 forks source link

Project model get_absolute_url() has side effect #4

Open raphaelyancey opened 6 years ago

raphaelyancey commented 6 years ago

https://github.com/Ircam-Web/mezzanine-organization/blob/2929e4d62663ee10394b1dae25c239399a2ec0a7/organization/projects/models.py#L99-L105

Line 101, it creates a record in the database whereas the method only aims to return a URL. Plus, the "ICT" topic seems pretty specific.

Wouldn't something like this do the trick ? Will do the PR if it does.

def get_absolute_url(self):
    ict_topic = ProjectTopic.objects.filter(key='ICT').first() # <ICT topic> or <None>
    if ict_topic is not None and self.topic == ict_topic:
        return reverse("organization-ict-project-detail", kwargs={"slug": self.slug})
    return reverse("organization-project-detail", kwargs={"slug": self.slug})
yomguy commented 6 years ago

Good catch! This is a really bad non-generic implementation made in the context of the Vertigo project :( Let's do (before I manage this in the views):

def get_absolute_url(self):
    topic = ProjectTopic.objects.filter(key='ICT').first() # <ICT topic> or <None>
    if topic and self.topic == topic:
        return reverse("organization-ict-project-detail", kwargs={"slug": self.slug})
    return reverse("organization-project-detail", kwargs={"slug": self.slug})

thanks

PS: please avoid is not None, just test the variable ;)