Skip to content

Fix spurious new-assessment log entries for renamed articles#1126

Open
arnchlmcodes wants to merge 4 commits intoopenzim:mainfrom
arnchlmcodes:fixes-issue72
Open

Fix spurious new-assessment log entries for renamed articles#1126
arnchlmcodes wants to merge 4 commits intoopenzim:mainfrom
arnchlmcodes:fixes-issue72

Conversation

@arnchlmcodes
Copy link
Copy Markdown
Contributor

Heyy @audiodude During update_project_assessments_by_kind, any article whose ref is absent from old_ratings gets treated as new. The problem is that a renamed article will always look new at this stage because the old name is what's stored in old_ratings, not the new one. Previously, store_new_ratings would immediately call add_log_for_rating for these, writing a spurious log entry with old_rating_value defaulting to NotA-Class.

The fix defers those log entries into a list instead of writing them immediately. Only articles that already existed in old_ratings but had a rating change get logged right away, since an existing ref changing its rating cannot be a rename.

process_unseen_articles runs after both quality and importance passes complete. For each old ref that wasn't seen, it calls get_move_data() to check if the article was renamed. If a move is detected, the destination ref is added to a moved_articles set. Back in update_project_assessments, the deferred logs are then written only for refs not present in moved_articles, ensuring that renamed articles don't get a spurious new-assessment log entry while genuinely new articles are logged correctly.

fixes issue #72

let me know if the fix is correct, I'll start working on the test cases next.

Copy link
Copy Markdown
Member

@audiodude audiodude left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This looks great! Please add the tests now.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants