Skip to content

Commit

Permalink
Avoid creating course user wiki timeslices for the complete universe …
Browse files Browse the repository at this point in the history
…of timeslices. Instead of that, we only create timeslices for non-empty course user wiki timeslices.
  • Loading branch information
gabina committed Dec 23, 2024
1 parent 4a33761 commit 52eaaee
Show file tree
Hide file tree
Showing 6 changed files with 83 additions and 111 deletions.
19 changes: 12 additions & 7 deletions app/models/course_user_wiki_timeslice.rb
Original file line number Diff line number Diff line change
Expand Up @@ -47,18 +47,23 @@ class CourseUserWikiTimeslice < ApplicationRecord
def self.update_course_user_wiki_timeslices(course, user_id, wiki, revisions)
rev_start = revisions[:start]
rev_end = revisions[:end]
# Course user wiki timeslices to update
course_user_wiki_timeslices = CourseUserWikiTimeslice.for_course_user_and_wiki(course,
user_id,
wiki)
.for_revisions_between(rev_start, rev_end)
course_user_wiki_timeslices.each do |timeslice|
# Timeslice dates to update are determined based on course wiki timeslices
timeslices = course.course_wiki_timeslices.where(wiki:)
.for_revisions_between(rev_start, rev_end)
timeslices.each do |timeslice|
# Group revisions that belong to the timeslice
revisions_in_timeslice = revisions[:revisions].select do |revision|
timeslice.start <= revision.date && revision.date < timeslice.end
end
# Get or create article course timeslice based on course, article_id,
# timeslice.start and timeslice.end
cu_timeslice = CourseUserWikiTimeslice.find_or_create_by(course:,
user_id:,
wiki:,
start: timeslice.start,
end: timeslice.end)
# Update cache for CourseUserWikiTimeslice
timeslice.update_cache_from_revisions revisions_in_timeslice
cu_timeslice.update_cache_from_revisions revisions_in_timeslice
end
end

Expand Down
4 changes: 0 additions & 4 deletions app/services/update_timeslices_course_user.rb
Original file line number Diff line number Diff line change
Expand Up @@ -35,10 +35,6 @@ def run

def add_user_ids(user_ids)
return if user_ids.empty?
course_user_ids = @course.courses_users.where(user: user_ids)
# Create course user wiki timeslices
@timeslice_manager.create_timeslices_for_new_course_user_records course_user_ids
return unless @course.was_course_ever_updated?

@course.wikis.each do |wiki|
fetch_users_revisions_for_wiki(wiki, user_ids, @course.start, @course.end)
Expand Down
56 changes: 3 additions & 53 deletions lib/timeslice_manager.rb
Original file line number Diff line number Diff line change
Expand Up @@ -85,27 +85,21 @@ def delete_course_user_wiki_timeslices_after_end_date
end
end

# Creates course user timeslices records for every course wiki for new course users
# Takes an array of CoursesUsers records
def create_timeslices_for_new_course_user_records(courses_users)
create_empty_course_user_wiki_timeslices(start_dates, courses_users:)
end

# Creates course wiki timeslices records for new course wikis
# Creates course user timeslices records for new course wiki
# Takes a collection of Wikis
def create_timeslices_for_new_course_wiki_records(wikis)
courses_wikis = @course.courses_wikis.where(wiki: wikis)
create_empty_course_wiki_timeslices(start_dates, courses_wikis)
create_empty_course_user_wiki_timeslices(start_dates, courses_wikis:)
# create_empty_course_user_wiki_timeslices(start_dates, courses_wikis:)
end

# Creates course wiki timeslices records for missing timeslices due to a change in the start date
# Creates course user wiki timeslices records for missing timeslices
def create_timeslices_for_new_course_start_date
courses_wikis = @course.courses_wikis
# order matters
create_empty_course_user_wiki_timeslices(start_dates_backward)
# create_empty_course_user_wiki_timeslices(start_dates_backward)
create_empty_course_wiki_timeslices(start_dates_backward, courses_wikis, needs_update: true)
end

Expand All @@ -114,7 +108,7 @@ def create_timeslices_for_new_course_start_date
def create_timeslices_up_to_new_course_end_date
courses_wikis = @course.courses_wikis
# order matters
create_empty_course_user_wiki_timeslices(start_dates_from_old_end)
# create_empty_course_user_wiki_timeslices(start_dates_from_old_end)
create_empty_course_wiki_timeslices(start_dates_from_old_end, courses_wikis, needs_update: true)
end

Expand Down Expand Up @@ -203,50 +197,6 @@ def update_course_wiki_timeslices_that_need_update(wikis_and_starts)

private

# Creates empty article course timeslices
# Takes an array like the following:
# [{:article_id=>115, :course_id=>72},..., {:article_id=>116, :course_id=>72}]
def create_empty_article_course_timeslices(starts, articles_courses)
new_records = starts.map do |start|
articles_courses.map do |a_c|
tracked = a_c[:tracked].nil? || a_c[:tracked]
{ article_id: a_c[:article_id], course_id: a_c[:course_id], start:,
end: start + @course.timeslice_duration, tracked: }
end
end.flatten

return if new_records.empty?
# Do this in batches to avoid running the MySQL server out of memory
new_records.each_slice(5000) do |new_record_slice|
ArticleCourseTimeslice.import new_record_slice, on_duplicate_key_ignore: true
end
end

# Creates empty course user wiki timeslices
def create_empty_course_user_wiki_timeslices(starts, courses_users: nil, courses_wikis: nil)
# Only create course user wiki timeslices for students
courses_users ||= @course.courses_users.where(role: CoursesUsers::Roles::STUDENT_ROLE)
courses_wikis ||= @course.courses_wikis
new_records = starts.map do |start|
courses_users.map do |c_u|
courses_wikis.map do |c_w|
{ course_id: c_u.course_id, user_id: c_u.user_id, wiki_id: c_w.wiki_id, start:,
end: start + @course.timeslice_duration }
end
end
end.flatten

import_new_course_user_wiki_records new_records
end

def import_new_course_user_wiki_records(new_records)
return if new_records.empty?
# Do this in batches to avoid running the MySQL server out of memory
new_records.each_slice(5000) do |new_record_slice|
CourseUserWikiTimeslice.import new_record_slice, on_duplicate_key_ignore: true
end
end

# Creates empty course wiki timeslices
def create_empty_course_wiki_timeslices(starts, courses_wikis, needs_update: false)
new_records = starts.map do |start|
Expand Down
65 changes: 29 additions & 36 deletions spec/lib/timeslice_manager_spec.rb
Original file line number Diff line number Diff line change
Expand Up @@ -35,31 +35,14 @@
course.reload
end

describe '#create_timeslices_for_new_course_user_records' do
context 'when there are new courses users' do
it 'creates course user wiki timeslices for every wiki for the entire course' do
expect(course.course_user_wiki_timeslices.size).to eq(0)
timeslice_manager.create_timeslices_for_new_course_user_records(
new_course_users
)
course.reload
expect(course.course_user_wiki_timeslices.size).to eq(666)
expect(course.course_user_wiki_timeslices.min_by(&:start).start.to_date)
.to eq(Date.new(2024, 1, 1))
expect(course.course_user_wiki_timeslices.max_by(&:start).start.to_date)
.to eq(Date.new(2024, 4, 20))
end
end
end

describe '#create_course_wiki_timeslices_for_new_records' do
before do
create(:courses_wikis, wiki: wikibooks, course:)
course.reload
end

context 'when there are new courses wikis' do
it 'creates course wiki and course user wiki timeslices for the entire course' do
it 'creates course wiki timeslices for the entire course' do
# No course wiki timeslices exist previously
expect(course.course_wiki_timeslices.size).to eq(0)
timeslice_manager.create_timeslices_for_new_course_wiki_records([enwiki,
Expand All @@ -69,11 +52,8 @@
expect(course.course_wiki_timeslices.first.wiki).to eq(enwiki)
expect(course.course_wiki_timeslices.last.wiki).to eq(wikibooks)
expect(course.course_wiki_timeslices.size).to eq(222)
# Create all the course user wiki timeslices for the existing course users with student role
# for the new wiki
expect(course.course_user_wiki_timeslices.first.wiki).to eq(enwiki)
expect(course.course_user_wiki_timeslices.last.wiki).to eq(wikibooks)
expect(course.course_user_wiki_timeslices.size).to eq(444)

expect(course.course_user_wiki_timeslices.size).to eq(0)
end
end
end
Expand All @@ -91,14 +71,12 @@
context 'when the start date changed to a previous date' do
it 'creates timeslices for the missing period that needs_update' do
expect(course.course_wiki_timeslices.size).to eq(333)
expect(course.course_user_wiki_timeslices.size).to eq(666)

timeslice_manager.create_timeslices_for_new_course_start_date
course.reload
# Create course and user timeslices for the period between 2023-12-20 and 2024-01-01
expect(course.course_wiki_timeslices.size).to eq(369)
expect(course.course_wiki_timeslices.where(needs_update: true).size).to eq(36)
expect(course.course_user_wiki_timeslices.size).to eq(738)
end
end
end
Expand All @@ -116,29 +94,28 @@
context 'when the end date changed to a later date' do
it 'creates timeslices for the missing period that needs_update' do
expect(course.course_wiki_timeslices.size).to eq(333)
expect(course.course_user_wiki_timeslices.size).to eq(666)

timeslice_manager.create_timeslices_up_to_new_course_end_date
course.reload
# Create course and user timeslices for the period between 2024-04-20 and 2024-04-30
expect(course.course_wiki_timeslices.size).to eq(363)
expect(course.course_wiki_timeslices.where(needs_update: true).size).to eq(30)
expect(course.course_user_wiki_timeslices.size).to eq(726)
end
end
end

describe '#delete_course_user_timeslices_for_deleted_course_users' do
before do
timeslice_manager.create_timeslices_for_new_course_user_records(new_course_users)
course.reload
create(:course_user_wiki_timeslice, course:, user_id: 1, wiki: enwiki)
create(:course_user_wiki_timeslice, course:, user_id: 2, wiki: enwiki)
create(:course_user_wiki_timeslice, course:, user_id: 3, wiki: enwiki)
end

it 'deletes course user wiki timeslices for the given users properly' do
expect(course.course_user_wiki_timeslices.size).to eq(666)
expect(course.course_user_wiki_timeslices.size).to eq(3)

timeslice_manager.delete_course_user_timeslices_for_deleted_course_users([1, 2])
expect(course.course_user_wiki_timeslices.size).to eq(222)
expect(course.course_user_wiki_timeslices.size).to eq(1)
end
end

Expand All @@ -148,6 +125,9 @@
timeslice_manager.create_timeslices_for_new_course_wiki_records([wikibooks,
wikidata,
enwiki])
create(:course_user_wiki_timeslice, course:, user_id: 1, wiki: wikibooks)
create(:course_user_wiki_timeslice, course:, user_id: 1, wiki: wikidata)
create(:course_user_wiki_timeslice, course:, user_id: 1, wiki: enwiki)
create(:article_course_timeslice, course:, article: article1)
create(:article_course_timeslice, course:, article: article2)
create(:article_course_timeslice, course:, article: article3)
Expand All @@ -156,12 +136,13 @@

it 'deletes wiki timeslices for the entire course properly' do
expect(course.course_wiki_timeslices.size).to eq(333)
expect(course.course_user_wiki_timeslices.size).to eq(666)
expect(course.course_user_wiki_timeslices.size).to eq(3)
expect(course.article_course_timeslices.size).to eq(3)

timeslice_manager.delete_timeslices_for_deleted_course_wikis([wikibooks.id,
wikidata.id])
course.reload
expect(course.course_user_wiki_timeslices.size).to eq(1)
# Course wiki timeslices for wikibooks and wikidata were deleted
expect(course.course_wiki_timeslices.where(wiki_id: wikibooks.id).size).to eq(0)
expect(course.course_wiki_timeslices.where(wiki_id: wikidata.id).size).to eq(0)
Expand Down Expand Up @@ -223,19 +204,25 @@
timeslice_manager.create_timeslices_for_new_course_wiki_records([wikibooks,
wikidata,
enwiki])
create(:course_user_wiki_timeslice, course:, user_id: 1, wiki: enwiki,
start: '2024-01-08'.to_datetime, end: '2024-01-09'.to_datetime)
create(:course_user_wiki_timeslice, course:, user_id: 1, wiki: enwiki,
start: '2024-01-10'.to_datetime, end: '2024-01-11'.to_datetime)
create(:course_user_wiki_timeslice, course:, user_id: 1, wiki: enwiki,
start: '2024-01-11'.to_datetime, end: '2024-01-12'.to_datetime)
course.reload
end

it 'deletes course user wiki timeslices for dates prior to start date properly' do
expect(course.course_user_wiki_timeslices.size).to eq(666)
expect(course.course_user_wiki_timeslices.size).to eq(3)

# Update course start date
course.update(start: '2024-01-10'.to_datetime)
timeslice_manager.delete_course_user_wiki_timeslices_prior_to_start_date
course.reload

# Course user wiki timeslices prior to the new start date were deleted
expect(course.course_user_wiki_timeslices.size).to eq(612)
expect(course.course_user_wiki_timeslices.size).to eq(2)
end
end

Expand All @@ -245,18 +232,24 @@
timeslice_manager.create_timeslices_for_new_course_wiki_records([wikibooks,
wikidata,
enwiki])
create(:course_user_wiki_timeslice, course:, user_id: 1, wiki: enwiki,
start: '2024-01-08'.to_datetime, end: '2024-01-09'.to_datetime)
create(:course_user_wiki_timeslice, course:, user_id: 1, wiki: enwiki,
start: '2024-04-10'.to_datetime, end: '2024-04-11'.to_datetime)
create(:course_user_wiki_timeslice, course:, user_id: 1, wiki: enwiki,
start: '2024-04-11'.to_datetime, end: '2024-04-12'.to_datetime)
end

it 'deletes course user wiki timeslices for dates after the end date properly' do
expect(course.course_user_wiki_timeslices.size).to eq(666)
expect(course.course_user_wiki_timeslices.size).to eq(3)

# Update course start date
course.update(end: '2024-04-10'.to_datetime)
timeslice_manager.delete_course_user_wiki_timeslices_after_end_date
course.reload

# Course user wiki timeslices prior to the new start date were deleted
expect(course.course_user_wiki_timeslices.size).to eq(606)
expect(course.course_user_wiki_timeslices.size).to eq(2)
end
end

Expand Down
30 changes: 27 additions & 3 deletions spec/models/course_user_wiki_timeslice_spec.rb
Original file line number Diff line number Diff line change
Expand Up @@ -21,6 +21,7 @@
#

require 'rails_helper'
require "#{Rails.root}/lib/timeslice_manager"

describe CourseUserWikiTimeslice, type: :model do
let(:wiki) { Wiki.get_or_create(language: 'en', project: 'wikipedia') }
Expand Down Expand Up @@ -107,18 +108,40 @@

describe '.update_course_user_wiki_timeslices' do
before do
TimesliceManager.new(course).create_timeslices_for_new_course_wiki_records(course.wikis)
revisions << build(:revision, article:, user_id: user.id, date: start + 26.hours)
revisions << build(:revision, article:, user_id: user.id, date: start + 50.hours)
revisions << build(:revision, article:, user_id: user.id, date: start + 51.hours)
end

it 'creates the right article timeslices based on the revisions' do
expect(course.course_user_wiki_timeslices.count).to eq(0)

start_period = start.strftime('%Y%m%d%H%M%S')
end_period = (start + 55.hours).strftime('%Y%m%d%H%M%S')
revision_data = { start: start_period, end: end_period, revisions: }
described_class.update_course_user_wiki_timeslices(course, user.id, wiki, revision_data)

course_user_wiki_timeslice_0 = described_class.find_by(course:, wiki:, user:, start:)
course_user_wiki_timeslice_1 = described_class.find_by(course:, wiki:, user:,
start: start + 1.day)
course_user_wiki_timeslice_2 = described_class.find_by(course:, wiki:, user:,
start: start + 2.days)

expect(course_user_wiki_timeslice_0.revision_count).to eq(4)
expect(course_user_wiki_timeslice_1.revision_count).to eq(1)
expect(course_user_wiki_timeslice_2.revision_count).to eq(2)
expect(course.course_user_wiki_timeslices.count).to eq(3)
end

it 'updates the right article timeslices based on the revisions' do
# Timeslices are already created
create(:course_user_wiki_timeslice, course:, wiki:, user:, start:, end: start + 1.day)
create(:course_user_wiki_timeslice, course:, wiki:, user:, start: start + 1.day,
end: start + 2.days)
create(:course_user_wiki_timeslice, course:, wiki:, user:, start: start + 2.days,
end: start + 3.days)
end

it 'updates the right article timeslices based on the revisions' do
course_user_wiki_timeslice_0 = described_class.find_by(course:, wiki:, user:, start:)
course_user_wiki_timeslice_1 = described_class.find_by(course:, wiki:, user:,
start: start + 1.day)
Expand All @@ -132,7 +155,7 @@
start_period = start.strftime('%Y%m%d%H%M%S')
end_period = (start + 55.hours).strftime('%Y%m%d%H%M%S')
revision_data = { start: start_period, end: end_period, revisions: }
described_class.update_course_user_wiki_timeslices(course, user, wiki, revision_data)
described_class.update_course_user_wiki_timeslices(course, user.id, wiki, revision_data)

course_user_wiki_timeslice_0 = described_class.find_by(course:, wiki:, user:, start:)
course_user_wiki_timeslice_1 = described_class.find_by(course:, wiki:, user:,
Expand All @@ -143,6 +166,7 @@
expect(course_user_wiki_timeslice_0.revision_count).to eq(4)
expect(course_user_wiki_timeslice_1.revision_count).to eq(1)
expect(course_user_wiki_timeslice_2.revision_count).to eq(2)
expect(course.course_user_wiki_timeslices.count).to eq(3)
end
end

Expand Down
Loading

0 comments on commit 52eaaee

Please sign in to comment.