[Date Prev][Date Next] [Thread Prev][Thread Next] [Date Index] [Thread Index]

Bug#754658: please display the package's description



Hi,

On Fri, 29 Aug 2014, Andrew Starr-Bochicchio wrote:
> This is done in distro_tracker/core/views.py Let me know if there is a
> better place. I know that views.py can become a bit of a dumping
> ground, but I didn't see anywhere that made more sense.

It should probably be a method of PackageName so that we get the short
description for free in any other context... for example in the mail
bot when we confirm the subscription or something like that.

> Also this should really test both if the names match and if they
> don't, but for the life of me I can't figure out how to get that
> working in the test suite. I was trying something like:
> 
>     def test_short_desc_on_page_when_same(self):
>         """
>         Tests that the short description is displayed when the source package
>         and binary package have the same name.
>         """
>         package = SourcePackageName.objects.create(name='another-package')
>         src_pkg = SourcePackage.objects.create(
>             source_package_name=package, version='2.0.0')
>         binary_package = BinaryPackageName.objects.create(
>             name='another-package')
>         bin_pkg = BinaryPackage.objects.create(
>             binary_package_name=binary_package,
>             source_package=src_pkg,
>             short_description='another useful package')
>         src_pkg.binary_packages = [binary_package]
>         src_pkg.save()

Use .get_or_create() instead of create and it will work. SourcePackageName
and BinaryPackageName are proxy models of PackageName so they share the
same database table and you can't create it twice... the second
.get_or_create() will actually do a .get() + a small update to mark the
name as a name of a binary package too...

> There must be a way to do it but I'm starting to get a bit frustrated,
> so I figured I'd just ask. =)

Sure! :)

Some comments:
- add a test to ensure that it works also when the database doesn't
  contain any information about binary packages (it's quite possible to
  configure distro-tracker that way)
- please rebase the patch on top of the latest git version, there was
  quite a bit of churn due to stylistich changes...
- make sure you can run "tox" without failures (I run it on a sid-based
  system, but jessie will do as well)

> +    try:
> +        bin_pkg = BinaryPackage.objects.filter(binary_package_name=package)
> +        desc = bin_pkg.first().short_description
> +    except AttributeError:
> +        source = SourcePackage.objects.filter(source_package_name=package)
> +        bin_pkg_name = source.last().binary_packages.first()
> +        bin_pkg = BinaryPackage.objects.filter(binary_package_name=bin_pkg_name)
> +        desc = bin_pkg.first().short_description

As per-comment above, it will likely fail if there are no binary packages
in the database.

Also it would be nice if this could be written in a way that doesn't
generate a new SQL request but instead relies on attributes of "package"
(that should be prefetched when we fetch that object).

https://docs.djangoproject.com/en/1.7/ref/models/querysets/#prefetch-related

(The whole webpage is highly non-optimized and peeks at something like 58
SQL requests! We should get that down to less than 10 IMO.)

>      return render(request, 'core/package.html', {
>          'package': package,
> +        'short_description': desc,
>          'panels': get_panels_for_package(package, request),
>          'is_subscribed': is_subscribed,
>      })

If we use a method of package, then you don't have to inject it here.

Cheers,
-- 
Raphaël Hertzog ◈ Debian Developer

Discover the Debian Administrator's Handbook:
→ http://debian-handbook.info/get/


Reply to: