From a060f5840c950efcff9124e6a1631224f2797f53 Mon Sep 17 00:00:00 2001 From: TLSM Date: Wed, 15 Mar 2023 01:00:22 -0400 Subject: [PATCH] fix: comment score regression MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit PR #547 introduced a UI bug where the comment score tooltip would show the wrong values (and in fact, the displayed score was also wrong, though this was not originally noticed). Specifically, a comment with e.g. +4 | -1 would display in the tooltip as being +4 | -4 and display a score of 4. The desired behavior would be +4 | -1 and score of 3. Precisely, the upvote value was displayed for each of upvotes, downvotes, and net score. Root cause was the `@lazy` decorator at: `files.classes.comment._score_context_str(⋅)` `@lazy` is very dumb. I don't entirely know why we don't just use `functools.cache`, but we use `@lazy` everywhere. It is entirely ignorant of the parameters to a function--not a substitute for memoization. comments.html contains the following snippet: {%- set ups = c.upvotes_str(render_ctx) -%} {%- set score = c.score_str(render_ctx) -%} {%- set downs = c.downvotes_str(render_ctx) -%} Each of those three functions internally calls to `_score_context_str` but with different arguments. The first call to upvotes gets cached by `@lazy` and the two subsequent calls get the upvotes string, rather than what they wanted. It's a cheap enough operation that it's not really worth memoizing, so we just remove the decorator. --- files/classes/comment.py | 3 +-- 1 file changed, 1 insertion(+), 2 deletions(-) diff --git a/files/classes/comment.py b/files/classes/comment.py index 0aceb7dcb..a916ee46d 100644 --- a/files/classes/comment.py +++ b/files/classes/comment.py @@ -91,13 +91,12 @@ class Comment(Base): comment_age_hours = comment_age_seconds / (60*60) return comment_age_hours < app.config['SCORE_HIDING_TIME_HOURS'] - @lazy def _score_context_str(self, score_type:Literal['score', 'upvotes', 'downvotes'], context:CommentRenderContext) -> str: if self.is_message: return '' # don't show scores for messages if context == 'volunteer': return '' # volunteer: hide scores if self.should_hide_score: return '' # hide scores for new comments - + if score_type == 'upvotes': return str(self.upvotes) if score_type == 'score': return str(self.score) if score_type == 'downvotes': return str(self.downvotes)