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

Some companies listed as related to themselves #1101

Closed
baltpeter opened this issue Sep 19, 2023 · 3 comments · Fixed by #1102
Closed

Some companies listed as related to themselves #1101

baltpeter opened this issue Sep 19, 2023 · 3 comments · Fixed by #1102
Assignees
Labels
bug Something isn't working

Comments

@baltpeter
Copy link
Member

We've received a report that some companies are listed as related to themselves, e.g. https://www.datarequests.org/company/ndr/:

image

@baltpeter baltpeter added the bug Something isn't working label Sep 19, 2023
@baltpeter baltpeter self-assigned this Sep 19, 2023
@baltpeter
Copy link
Member Author

This surprised me since we do already have a condition in place that should prevent this exact problem:

{{ if and (ne .Permalink $.Permalink) (not .Params.nsfw) }}

@baltpeter
Copy link
Member Author

Debugging this, I found that $.Permalink was wrong (for a different page).

That makes sense! We are heavily caching the related companies calculation (an optimization that is very necessary as explained in the commit message of bce6bff):

{{ partialCached "related-companies.html" . .Params.categories (.Param "relevant-countries") }}

However, we are only varying the cache based on the categories and relevant countries. Thus, we will get the wrong permalink for cached renders, of course.

Unfortunately, we can't just also pass the permalink to partialCached. That would mean that nothing gets cached anymore.

@baltpeter
Copy link
Member Author

Solution is a bit annoying but not too bad: We make the related companies partial into a function that returns an array of the candidates and then do the filtering on each individual page. Only question is how much of a performance impact that is, so let's do some benchmarking. Running on my machine (I ran each one multiple times to hopefully negate disk caching fluctuations):

  • production
    • before: 19219 ms, 19005 ms
    • after: 19405 ms, 19181 ms
  • dev
    • before: 14565 ms, 14393 ms
    • after: 14907 ms, 14685 ms

Looking good!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Something isn't working
Development

Successfully merging a pull request may close this issue.

1 participant