[HNT-405] Remove blocked topics from sections

[mmiermans]

References

JIRA: HNT-405

Description

If a user blocks a section using the Follow/Block feature (e.g. Food, Politics), we remove recommendations with those topics from sections, including 'Popular Today'.

PR Review Checklist

Put an x in the boxes that apply

• This PR conforms to the Contribution Guidelines
• The PR title starts with the JIRA issue reference, format example [DISCO-####], and has the same title (if applicable)
• [load test: (abort|skip|warn)] keywords are applied to the last commit message (if applicable)
• Documentation has been updated (if applicable)
• Functional and performance test coverage has been expanded and maintained (if applicable)

[jpetto]

i think it's worth it to cover more scenarios in some unit tests.

[jpetto]

i think it would be nice to abstract this to a helper function with a few unit tests.

• verify it's only removing recommendations matching a blocked section
• verify it does nothing if no sections are marked as blocked
• verify it handles recommendations without topics as expected (is the expectation that a recommendation without a topic be excluded here? also how do recommendations without a topic get in?)

[jpetto]

also - do you think it's worth skipping the list comprehension (i think that's what it's called?) on line 329 if blocked_section_ids has a length of 0?

[mmiermans]

Good question. I was curious what the performance impact was, so I did a quick benchmark:

import timeit
setup_code = '''
class DummyTopic:
    def __init__(self, value):
        self.value = value
class DummyRec:
    def __init__(self, topic):
        self.topic = topic
# Create 10,000 dummy recommendations.
# For this benchmark, each recommendation has a DummyTopic.
recommendations = [DummyRec(DummyTopic("dummy")) for _ in range(1000)]
# blocked_section_ids is empty.
blocked_section_ids = set()
'''
stmt = '''
filtered = [r for r in recommendations if not r.topic or r.topic.value not in blocked_section_ids]
'''
# Run the list comprehension 1000 times and measure the total time.
iterations = 1000
total_time = timeit.timeit(stmt=stmt, setup=setup_code, number=iterations)
avg_time_ms = (total_time / iterations) * 1000
print(f"Average time per iteration: {avg_time_ms:.3f} ms")

Average time per iteration: 0.028 ms

Currently we have a few hundred recommendations. At the cost of a few microseconds, I would lean towards letting list comprehension handle the case of an empty list.

[jpetto]

just to say - if the new logic in the above file is fully tested in a unit test, the only line of code this function would really be testing (that isn't already tested) is line 327 in the file above:

if request.sections:

for peace of mind this test isn't bad though.

[jpetto]

looks much better. great tests and use of a static method!

[jpetto]

nice use of a static method!