[Data rearchitecture] General clean up: remove old comments, TODOs, a…

…nd unused code (#6169)

* Deletes CleanArticlesCoursesWorke because update_from_course is already being called

* Stop importing wikidata summaries in daily update.

* Removed unnecessary TODO comments

* Remove commented code

app/services/update_course_stats_timeslice.rb

@@ -45,13 +45,11 @@ def initialize(course)
 
   def import_uploads
     @debugger.log_update_progress :start
-    # TODO: note this is not wiki scoped.
     CourseUploadImporter.new(@course, update_service: self).run
     @debugger.log_update_progress :uploads_imported
   end
 
   def update_categories
-    # TODO: note this is not wiki scoped.
     Category.refresh_categories_for(@course)
     @debugger.log_update_progress :categories_updated
   end
@@ -62,7 +60,6 @@ def update_article_status
   end
 
   def update_average_pageviews
-    # TODO: note this is not wiki scoped.
     AverageViewsImporter.update_outdated_average_views(@course.articles)
     @debugger.log_update_progress :average_pageviews_updated
   end
@@ -107,7 +104,6 @@ def update_wiki_namespace_stats
   def log_end_of_update
     @course.update(needs_update: false)
     @end_time = Time.zone.now
-    # TODO: improve the course flag updates
     UpdateLogger.update_course(@course, 'start_time' => @start_time.to_datetime,
                                          'end_time' => @end_time.to_datetime,
                                          'sentry_tag_uuid' => sentry_tag_uuid,

app/services/update_course_wiki_timeslices.rb

@@ -96,11 +96,6 @@ def fetch_data(wiki, timeslice_start, timeslice_end)
     wikidata_revisions = @revisions[wiki][:revisions].reject(&:deleted)
     @revisions[wiki][:revisions] =
       @wikidata_stats_updater.update_revisions_with_stats(wikidata_revisions)
-    # TODO: replace the logic on ArticlesCourses.update_from_course to remove all
-    # the ArticlesCourses that do not correspond to course revisions.
-    # That may happen if the course dates changed, so some revisions are no
-    # longer part of the course.
-    # Also remove records for articles that aren't on a tracked wiki.
   end
 
   def process_timeslices(wiki)
@@ -128,9 +123,7 @@ def update_article_course_timeslices_for_wiki(revisions)
     end_period = revisions[:end]
     revs = revisions[:revisions]
     revs.group_by(&:article_id).each do |article_id, article_revisions|
-      # We don't create articles courses for every article
-      # article_course = ArticlesCourses.find_by(course: @course, article_id:)
-      # next unless article_course
+      # We create articles courses for every article
 
       # Update cache for ArticleCourseTimeslice
       article_revisions_data = { start: start_period, end: end_period,

app/services/update_timeslices_course_user.rb

@@ -72,8 +72,6 @@ def remove_courses_users(user_ids)
     @timeslice_cleaner.reset_timeslices_that_need_update_from_article_timeslices(
       @article_course_timeslices_for_users
     )
-    # Clean articles courses timeslices
-    # clean_article_course_timeslices
     # Delete articles courses that were updated only for removed users
     ArticlesCoursesCleanerTimeslice.clean_articles_courses_for_user_ids(@course, user_ids)
   end
@@ -87,11 +85,4 @@ def get_article_course_timeslices_for_users(user_ids)
     # These are the ArticleCourseTimeslice records that were updated by users
     timeslices.flatten
   end
-
-  # def clean_article_course_timeslices
-  #   ids = @article_course_timeslices_for_users.map(&:id)
-  #   ArticleCourseTimeslice.where(id: ids).update_all(character_sum: 0,
-  #                                                    references_count: 0,
-  #                                                    user_ids: nil)
-  # end
 end

app/services/update_wikidata_stats.rb

@@ -1,6 +1,5 @@
 # frozen_string_literal: true
 require_dependency "#{Rails.root}/lib/wikidata_summary_parser"
-require_dependency "#{Rails.root}/lib/importers/wikidata_summary_importer"
 # require the installed wikidata-diff-analyzer gem
 require 'wikidata-diff-analyzer'
 

app/services/update_wikidata_stats_timeslice.rb

@@ -1,6 +1,5 @@
 # frozen_string_literal: true
 require_dependency "#{Rails.root}/lib/wikidata_summary_parser"
-require_dependency "#{Rails.root}/lib/importers/wikidata_summary_importer"
 # require the installed wikidata-diff-analyzer gem
 require 'wikidata-diff-analyzer'
 

lib/articles_courses_cleaner_timeslice.rb

@@ -84,8 +84,6 @@ def remove_articles_courses_for_dates_prior_to_start_date
 
     delete_article_course(article_ids_to_delete)
 
-    # NOTE: this could be implemented in the TimesliceManager class
-
     # Delete article course timeslices for deleted articles
     timeslice_ids = ArticleCourseTimeslice.where(course: @course)
                                           .where(article_id: article_ids_to_delete)
@@ -108,8 +106,6 @@ def remove_articles_courses_for_dates_after_end_date
 
     delete_article_course(article_ids_to_delete)
 
-    # NOTE: this could be implemented in the TimesliceManager class
-
     # Delete article course timeslices for deleted articles
     timeslice_ids = ArticleCourseTimeslice.where(course: @course)
                                           .where(article_id: article_ids_to_delete)
@@ -126,13 +122,6 @@ def remove_articles_courses_for_dates_after_end_date
     delete_article_course_timeslice_ids(timeslice_ids) unless timeslice_ids.nil?
   end
 
-  # Removes the articles courses and timeslices records for article ids.
-  def remove_articles_courses_for_article_ids(article_ids)
-    delete_article_course(article_ids)
-    timeslices = ArticleCourseTimeslice.where(course: @course).where(article_id: article_ids)
-    delete_article_course_timeslice_ids(timeslices.pluck(:id))
-  end
-
   def reset_deleted_or_untracked_articles
     # Note that this could remove articles courses records for manually untracked articles
     # Find articles with an articles_courses record but without a non-deleted article record.
@@ -172,7 +161,6 @@ def reset_undeleted_or_retracked_articles
   def reset(articles, wiki = nil)
     mark_as_needs_update(articles, wiki)
     delete_article_course(articles.pluck(:id))
-    # remove_articles_courses_for_article_ids(articles.pluck(:id))
   end
 
   private

lib/data_cycle/daily_update.rb

@@ -3,9 +3,7 @@
 require_dependency "#{Rails.root}/app/workers/daily_update/update_users_worker"
 require_dependency "#{Rails.root}/app/workers/daily_update/update_commons_uploads_worker"
 require_dependency "#{Rails.root}/app/workers/daily_update/find_assignments_worker"
-require_dependency "#{Rails.root}/app/workers/daily_update/clean_articles_courses_worker"
 require_dependency "#{Rails.root}/app/workers/daily_update/import_ratings_worker"
-require_dependency "#{Rails.root}/app/workers/daily_update/import_wikidata_summaries_worker"
 require_dependency "#{Rails.root}/app/workers/daily_update/overdue_training_alert_worker"
 require_dependency "#{Rails.root}/app/workers/daily_update/salesforce_sync_worker"
 require_dependency "#{Rails.root}/app/workers/daily_update/wiki_discouraged_article_worker"
@@ -35,7 +33,6 @@ def run_update
     update_commons_uploads
     update_article_data
     update_wiki_discouraged_article if Features.wiki_ed?
-    import_wikidata_summaries if Features.wiki_ed?
     send_term_recap_emails if Features.wiki_ed?
     generate_overdue_training_alerts if Features.wiki_ed?
     push_course_data_to_salesforce if Features.wiki_ed?
@@ -65,18 +62,10 @@ def update_article_data
     log_message 'Finding articles that match assignment titles'
     FindAssignmentsWorker.set(queue: QUEUE).perform_async
 
-    log_message 'Rebuilding ArticlesCourses for all current students'
-    CleanArticlesCoursesWorker.set(queue: QUEUE).perform_async
-
     log_message 'Updating ratings for all articles'
     ImportRatingsWorker.set(queue: QUEUE).perform_async
   end
 
-  def import_wikidata_summaries
-    log_message 'Importing Wikidata revision summaries'
-    ImportWikidataSummariesWorker.set(queue: QUEUE).perform_async
-  end
-
   def update_wiki_discouraged_article
     log_message 'Updating Wiki Education discouraged articles'
     WikiDiscouragedArticleWorker.set(queue: QUEUE).perform_async

spec/lib/course_cache_manager_spec.rb

@@ -79,7 +79,6 @@
 
     it 'updates uploads based on existing common uploads records' do
       described_class.new(course).update_cache_from_timeslices []
-      # TODO: modify to be calculated from course wiki timeslices values
       expect(course.upload_count).to eq(2)
       expect(course.uploads_in_use_count).to eq(2)
       expect(course.upload_usages_count).to eq(7)

spec/lib/data_cycle/daily_update_spec.rb

@@ -19,8 +19,6 @@
       worker_double = class_double(WikiDiscouragedArticleWorker)
       expect(UserImporter).to receive(:update_users)
       expect(AssignedArticleImporter).to receive(:import_articles_for_assignments)
-      # TODO: modify this when implementing the rebuilding articles courses without revisions
-      # expect(ArticlesCoursesCleaner).to receive(:rebuild_articles_courses)
       expect(RatingImporter).to receive(:update_all_ratings)
       expect(UploadImporter).to receive(:find_deleted_files)
       expect_any_instance_of(OverdueTrainingAlertManager).to receive(:create_alerts)