Skip to content

GitLab

  • Menu
Projects Groups Snippets
  • Help
    • Help
    • Support
    • Community forum
    • Submit feedback
    • Contribute to GitLab
  • Sign in
  • castellum castellum
  • Project information
    • Project information
    • Activity
    • Labels
    • Members
  • Repository
    • Repository
    • Files
    • Commits
    • Branches
    • Tags
    • Contributors
    • Graph
    • Compare
  • Issues 5
    • Issues 5
    • List
    • Boards
    • Service Desk
    • Milestones
  • Merge requests 10
    • Merge requests 10
  • CI/CD
    • CI/CD
    • Pipelines
    • Jobs
    • Schedules
  • Deployments
    • Deployments
    • Releases
  • Packages & Registries
    • Packages & Registries
    • Container Registry
  • Analytics
    • Analytics
    • Value stream
    • CI/CD
    • Repository
  • Activity
  • Graph
  • Create a new issue
  • Jobs
  • Commits
  • Issue Boards
Collapse sidebar
  • Castellum
  • castellumcastellum
  • Merge requests
  • !1533

Closed
Created Sep 14, 2020 by Bengfort@bengfortOwner
  • Report abuse
Report abuse

1766 showup in list

  • Overview 3
  • Commits 5
  • Pipelines 3
  • Changes 10

So the was supposed to be a simple template change to display showup in the execution list. However, I fell down the annotate/property rabbit hole.

The simple way to add a computed field to a model is by defining a @property. This works well and is easy to understand. However, it becomes a performance issue on lists if the property makes additional database requests.

The better option in that case is to calculate the field in the database using .annotate(). That is more complicated but can improve performance a lot.

The big trouble with .annotate() is that it is not available everywhere, notably instances that have not yet been saved to the database.

Another issue is that you have to decide: Either you add the annotation to a model's default manager which makes every single query more complicated, or you do the annotation on a case-by-case basis, which makes them unavailable in many situations.

The holy grail would be to combine both approaches: To have a single definitions that works and is efficient in all cases.

This MR could bring us a step closer to that goal: It adds the annotate_instance() helper that will get annotations from the database and add them to an existing instance, but only if they do not exist yet. It is based on this stackoverflow answer.

This helper is then used to efficiently render showup both on single subjects as well as on a pre-annotated list of subjects.

Scratch everything I wrote above. The list we need to show the showup in is not actually a list of subjects, but a list of participations. So the issue is that we cannot annotate a .select_related() query -- at least I don't know how. [.prefetch_related() did the trick]

So not the big question is: Do we want this?

  • Merge like this
  • Remove the optimization and just use @property instead
  • Do not show showup in the list at all
Edited Sep 14, 2020 by Bengfort
Assignee
Assign to
Reviewer
Request review from
Time tracking
Source branch: 1766-show-up-in-list