From e45323f785ccd8a2fc4151400438fdc7fb0e5ec4 Mon Sep 17 00:00:00 2001 From: Andrew Roth Date: Wed, 8 Dec 2021 12:33:44 -0500 Subject: [PATCH 01/15] implement saving user count values --- app/controllers/user_counters_controller.rb | 17 ++++- app/models/user_counter.rb | 9 ++- app/models/user_counter_value.rb | 3 + app/serializers/user_counter_serializer.rb | 5 ++ .../user_counter_value_serializer.rb | 3 + .../20211110153757_create_user_counters.rb | 2 +- ...211201161437_create_user_counter_values.rb | 11 ++++ db/schema.rb | 12 +++- .../user_counters_controller_spec.rb | 62 ++++++++++++++++++- spec/factories/user_counter_values.rb | 5 ++ 10 files changed, 120 insertions(+), 9 deletions(-) create mode 100644 app/models/user_counter_value.rb create mode 100644 app/serializers/user_counter_value_serializer.rb create mode 100644 db/migrate/20211201161437_create_user_counter_values.rb create mode 100644 spec/factories/user_counter_values.rb diff --git a/app/controllers/user_counters_controller.rb b/app/controllers/user_counters_controller.rb index 15ea56e23..0949a233a 100644 --- a/app/controllers/user_counters_controller.rb +++ b/app/controllers/user_counters_controller.rb @@ -3,10 +3,12 @@ class UserCountersController < WithUserController def update counter = current_user.user_counters.where(counter_name: counter_name).first_or_initialize + counter.decay increment = permitted_params[:increment].to_f counter.count += increment - counter.decay counter.decayed_count += increment + counter.save! if counter.new_record? # need to save before applying values if new user counter + apply_values(counter, permitted_params[:values]) if permitted_params[:values] counter.save! render json: counter, status: :ok end @@ -21,7 +23,7 @@ def index protected def permitted_params - params.require(:data).require(:attributes).permit(:increment) + params.require(:data).require(:attributes).permit(:increment, values: []) end def counter_name @@ -29,4 +31,15 @@ def counter_name counter_name += ".#{params[:format]}" if params[:format].present? counter_name end + + # todo: move to model + def apply_values(counter, values) + values_before = counter.values + new_values = values - values_before + new_values.each do |value| + counter.user_counter_values.create!(value: value) + end + counter.count += new_values.count + counter.decayed_count += new_values.count + end end diff --git a/app/models/user_counter.rb b/app/models/user_counter.rb index 0cc5f5a17..5a253c308 100644 --- a/app/models/user_counter.rb +++ b/app/models/user_counter.rb @@ -4,10 +4,11 @@ class UserCounter < ApplicationRecord DECAY_RATE = -0.0077 belongs_to :user + has_many :user_counter_values def decay - date_now = Date.today - self.last_decay ||= Date.today + date_now = Time.now.utc.to_date + self.last_decay ||= date_now days_to_decay = date_now - last_decay if days_to_decay > 0 @@ -15,4 +16,8 @@ def decay self.last_decay = date_now end end + + def values + user_counter_values.pluck(:value) + end end diff --git a/app/models/user_counter_value.rb b/app/models/user_counter_value.rb new file mode 100644 index 000000000..c2a44d1b9 --- /dev/null +++ b/app/models/user_counter_value.rb @@ -0,0 +1,3 @@ +class UserCounterValue < ApplicationRecord + belongs_to :user_counter +end diff --git a/app/serializers/user_counter_serializer.rb b/app/serializers/user_counter_serializer.rb index 46fde7d45..c22626686 100644 --- a/app/serializers/user_counter_serializer.rb +++ b/app/serializers/user_counter_serializer.rb @@ -3,8 +3,13 @@ class UserCounterSerializer < ActiveModel::Serializer type "user-counter" attributes :id, :count, :decayed_count, :last_decay + attribute :values, if: :has_values? def id object.counter_name end + + def has_values? + object.values.any? + end end diff --git a/app/serializers/user_counter_value_serializer.rb b/app/serializers/user_counter_value_serializer.rb new file mode 100644 index 000000000..f8032095a --- /dev/null +++ b/app/serializers/user_counter_value_serializer.rb @@ -0,0 +1,3 @@ +class UserCounterValueSerializer < ActiveModel::Serializer + attribute :value +end diff --git a/db/migrate/20211110153757_create_user_counters.rb b/db/migrate/20211110153757_create_user_counters.rb index 05fd399e0..34fe4c283 100644 --- a/db/migrate/20211110153757_create_user_counters.rb +++ b/db/migrate/20211110153757_create_user_counters.rb @@ -5,7 +5,7 @@ def change t.string :counter_name t.integer :count, default: 0 t.float :decayed_count, default: 0 - t.date :last_decay, default: -> { "NOW()" } + t.date :last_decay, default: -> { timezone('utc', NOW()) } t.timestamps end diff --git a/db/migrate/20211201161437_create_user_counter_values.rb b/db/migrate/20211201161437_create_user_counter_values.rb new file mode 100644 index 000000000..5efd074af --- /dev/null +++ b/db/migrate/20211201161437_create_user_counter_values.rb @@ -0,0 +1,11 @@ +class CreateUserCounterValues < ActiveRecord::Migration[6.0] + def change + create_table :user_counter_values do |t| + t.string :user_id + t.string :user_counter_id + t.string :value + + t.timestamps + end + end +end diff --git a/db/schema.rb b/db/schema.rb index 2184e9975..b3fd93580 100644 --- a/db/schema.rb +++ b/db/schema.rb @@ -10,7 +10,7 @@ # # It's strongly recommended that you check this file into your version control system. -ActiveRecord::Schema.define(version: 2021_11_10_153757) do +ActiveRecord::Schema.define(version: 2021_12_01_161437) do # These are extensions that must be enabled in order to support this database enable_extension "citext" @@ -226,12 +226,20 @@ t.index ["resource_id"], name: "index_translations_on_resource_id" end + create_table "user_counter_values", force: :cascade do |t| + t.string "user_id" + t.string "user_counter_id" + t.string "value" + t.datetime "created_at", precision: 6, null: false + t.datetime "updated_at", precision: 6, null: false + end + create_table "user_counters", force: :cascade do |t| t.integer "user_id" t.string "counter_name" t.integer "count", default: 0 t.float "decayed_count", default: 0.0 - t.date "last_decay", default: -> { "now()" } + t.date "last_decay", default: -> { "timezone('utc', NOW())" } t.datetime "created_at", precision: 6, null: false t.datetime "updated_at", precision: 6, null: false t.index ["user_id", "counter_name"], name: "index_user_counters_on_user_id_and_counter_name", unique: true diff --git a/spec/acceptance/user_counters_controller_spec.rb b/spec/acceptance/user_counters_controller_spec.rb index 4a949d4d7..d25950abf 100644 --- a/spec/acceptance/user_counters_controller_spec.rb +++ b/spec/acceptance/user_counters_controller_spec.rb @@ -57,12 +57,56 @@ expect(UserCounter.last.last_decay).to eq(Date.today) end end + + context "with values" do + it "create user_counter" do + expect { + do_request data: {type: "user_counter", attributes: {increment: 20, values: ["v1", "v2"]}} + }.to change { UserCounter.count }.by(1) + + expect(status).to eq(200) + json_response = JSON.parse(response_body)["data"] + expect(json_response).not_to be_nil + expect(json_response["id"]).to eq("tool_opens.kgp") + expect(json_response["attributes"]["count"]).to eq(22) + expect(json_response["attributes"]["decayed-count"]).to eq(22) + expect(json_response["attributes"]["last-decay"]).to eq(Date.today.to_s) + expect(UserCounter.last.counter_name).to eq("tool_opens.kgp") + expect(UserCounter.last.count).to eq(22) + expect(UserCounter.last.decayed_count).to eq(22) + expect(UserCounter.last.last_decay).to eq(Date.today) + end + + context "when user_counter exists" do + let!(:user_counter) { FactoryBot.create(:user_counter, user: user, counter_name: "tool_opens.kgp", count: 50, decayed_count: 50, last_decay: 90.days.ago) } + + it "updates the count and decay" do + expect { + do_request data: {type: "user_counter", attributes: {increment: 20, values: ["v1", "v2"]}} + }.to_not change { user_counter.count } + + expect(status).to eq(200) + json_response = JSON.parse(response_body)["data"] + expect(json_response).not_to be_nil + expect(json_response["id"]).to eq("tool_opens.kgp") + expect(json_response["attributes"]["count"]).to eq(72) + expect((json_response["attributes"]["decayed-count"] - 47).abs).to be <= 0.004 # look within 0.004, close enough + expect(json_response["attributes"]["last-decay"]).to eq(Date.today.to_s) + expect(json_response["attributes"]["values"]).to eq(["v1", "v2"]) + expect(UserCounter.last.count).to eq(72) + # get close to 45 -- original value of 50 should decay to 25 with the 90 day half-life, then +20 from the patch count incremement + expect((UserCounter.last.decayed_count - 47).abs).to be <= 0.004 + expect(UserCounter.last.last_decay).to eq(Date.today) + expect(UserCounter.last.values).to eq(["v1", "v2"]) + end + end + end end get "user/counters" do let(:user) { FactoryBot.create(:user) } - let!(:user_counter) { FactoryBot.create(:user_counter, user: user, counter_name: "tool_opens.kgp", count: 50, decayed_count: 50, last_decay: 90.days.ago) } - let!(:user_counter2) { FactoryBot.create(:user_counter, user: user, counter_name: "other.kgp", count: 60, decayed_count: 40, last_decay: 90.days.ago) } + let!(:user_counter) { FactoryBot.create(:user_counter, user: user, counter_name: "tool_opens.kgp", count: 50, decayed_count: 50, last_decay: 90.days.ago.utc) } + let!(:user_counter2) { FactoryBot.create(:user_counter, user: user, counter_name: "other.kgp", count: 60, decayed_count: 40, last_decay: 90.days.ago.utc) } requires_okta_login it "gets counts" do @@ -73,5 +117,19 @@ expected_result = %({"data":[{"id":"tool_opens.kgp","type":"user-counter","attributes":{"count":50,"decayed-count":25.00367978478838,"last-decay":"#{today}"}},{"id":"other.kgp","type":"user-counter","attributes":{"count":60,"decayed-count":20.002943827830705,"last-decay":"#{today}"}}]}) expect(response_body).to eq(expected_result) end + + context "with values" do + let!(:user_value1) { FactoryBot.create(:user_counter_value, user_counter: user_counter, value: "v1") } + let!(:user_value2) { FactoryBot.create(:user_counter_value, user_counter: user_counter, value: "v2") } + + it "includes values" do + do_request + + expect(status).to eq(200) + today = Date.today.to_s + expected_result = %({"data":[{"id":"tool_opens.kgp","type":"user-counter","attributes":{"count":50,"decayed-count":25.00367978478838,"last-decay":"#{today}","values":["v1","v2"]}},{"id":"other.kgp","type":"user-counter","attributes":{"count":60,"decayed-count":20.002943827830705,"last-decay":"#{today}"}}]}) + expect(response_body).to eq(expected_result) + end + end end end diff --git a/spec/factories/user_counter_values.rb b/spec/factories/user_counter_values.rb new file mode 100644 index 000000000..4ccbf0d45 --- /dev/null +++ b/spec/factories/user_counter_values.rb @@ -0,0 +1,5 @@ +FactoryBot.define do + factory :user_counter_value do + value { "value" } + end +end From 1006e90302beb5d11b640164bef1c52ad4b923d0 Mon Sep 17 00:00:00 2001 From: Andrew Roth Date: Tue, 14 Dec 2021 11:19:59 -0500 Subject: [PATCH 02/15] remove values from return --- app/serializers/user_counter_serializer.rb | 5 ----- db/migrate/20211110153757_create_user_counters.rb | 2 +- spec/acceptance/user_counters_controller_spec.rb | 3 +-- 3 files changed, 2 insertions(+), 8 deletions(-) diff --git a/app/serializers/user_counter_serializer.rb b/app/serializers/user_counter_serializer.rb index c22626686..46fde7d45 100644 --- a/app/serializers/user_counter_serializer.rb +++ b/app/serializers/user_counter_serializer.rb @@ -3,13 +3,8 @@ class UserCounterSerializer < ActiveModel::Serializer type "user-counter" attributes :id, :count, :decayed_count, :last_decay - attribute :values, if: :has_values? def id object.counter_name end - - def has_values? - object.values.any? - end end diff --git a/db/migrate/20211110153757_create_user_counters.rb b/db/migrate/20211110153757_create_user_counters.rb index 34fe4c283..f8f2e23cd 100644 --- a/db/migrate/20211110153757_create_user_counters.rb +++ b/db/migrate/20211110153757_create_user_counters.rb @@ -5,7 +5,7 @@ def change t.string :counter_name t.integer :count, default: 0 t.float :decayed_count, default: 0 - t.date :last_decay, default: -> { timezone('utc', NOW()) } + t.date :last_decay, default: -> { timezone("utc", NOW()) } t.timestamps end diff --git a/spec/acceptance/user_counters_controller_spec.rb b/spec/acceptance/user_counters_controller_spec.rb index d25950abf..7262f9538 100644 --- a/spec/acceptance/user_counters_controller_spec.rb +++ b/spec/acceptance/user_counters_controller_spec.rb @@ -92,7 +92,6 @@ expect(json_response["attributes"]["count"]).to eq(72) expect((json_response["attributes"]["decayed-count"] - 47).abs).to be <= 0.004 # look within 0.004, close enough expect(json_response["attributes"]["last-decay"]).to eq(Date.today.to_s) - expect(json_response["attributes"]["values"]).to eq(["v1", "v2"]) expect(UserCounter.last.count).to eq(72) # get close to 45 -- original value of 50 should decay to 25 with the 90 day half-life, then +20 from the patch count incremement expect((UserCounter.last.decayed_count - 47).abs).to be <= 0.004 @@ -127,7 +126,7 @@ expect(status).to eq(200) today = Date.today.to_s - expected_result = %({"data":[{"id":"tool_opens.kgp","type":"user-counter","attributes":{"count":50,"decayed-count":25.00367978478838,"last-decay":"#{today}","values":["v1","v2"]}},{"id":"other.kgp","type":"user-counter","attributes":{"count":60,"decayed-count":20.002943827830705,"last-decay":"#{today}"}}]}) + expected_result = %({"data":[{"id":"tool_opens.kgp","type":"user-counter","attributes":{"count":50,"decayed-count":25.00367978478838,"last-decay":"#{today}"}},{"id":"other.kgp","type":"user-counter","attributes":{"count":60,"decayed-count":20.002943827830705,"last-decay":"#{today}"}}]}) expect(response_body).to eq(expected_result) end end From 5c074180a0e616adff05be53ea640198fc34a1f6 Mon Sep 17 00:00:00 2001 From: Andrew Roth Date: Tue, 14 Dec 2021 13:06:46 -0500 Subject: [PATCH 03/15] switch to integer foreign keys --- db/migrate/20211201161437_create_user_counter_values.rb | 4 ++-- db/schema.rb | 4 ++-- 2 files changed, 4 insertions(+), 4 deletions(-) diff --git a/db/migrate/20211201161437_create_user_counter_values.rb b/db/migrate/20211201161437_create_user_counter_values.rb index 5efd074af..392a44ed1 100644 --- a/db/migrate/20211201161437_create_user_counter_values.rb +++ b/db/migrate/20211201161437_create_user_counter_values.rb @@ -1,8 +1,8 @@ class CreateUserCounterValues < ActiveRecord::Migration[6.0] def change create_table :user_counter_values do |t| - t.string :user_id - t.string :user_counter_id + t.integer :user_id + t.integer :user_counter_id t.string :value t.timestamps diff --git a/db/schema.rb b/db/schema.rb index b3fd93580..cfb47fbbf 100644 --- a/db/schema.rb +++ b/db/schema.rb @@ -227,8 +227,8 @@ end create_table "user_counter_values", force: :cascade do |t| - t.string "user_id" - t.string "user_counter_id" + t.integer "user_id" + t.integer "user_counter_id" t.string "value" t.datetime "created_at", precision: 6, null: false t.datetime "updated_at", precision: 6, null: false From 702d799382b847ea3ecc78d31a11d1520c4b4dc0 Mon Sep 17 00:00:00 2001 From: Andrew Roth Date: Wed, 8 Dec 2021 12:33:44 -0500 Subject: [PATCH 04/15] implement saving user count values --- app/controllers/user_counters_controller.rb | 17 ++++- app/models/user_counter.rb | 9 ++- app/models/user_counter_value.rb | 3 + app/serializers/user_counter_serializer.rb | 5 ++ .../user_counter_value_serializer.rb | 3 + .../20211110153757_create_user_counters.rb | 2 +- ...211201161437_create_user_counter_values.rb | 11 ++++ db/schema.rb | 12 +++- .../user_counters_controller_spec.rb | 62 ++++++++++++++++++- spec/factories/user_counter_values.rb | 5 ++ 10 files changed, 120 insertions(+), 9 deletions(-) create mode 100644 app/models/user_counter_value.rb create mode 100644 app/serializers/user_counter_value_serializer.rb create mode 100644 db/migrate/20211201161437_create_user_counter_values.rb create mode 100644 spec/factories/user_counter_values.rb diff --git a/app/controllers/user_counters_controller.rb b/app/controllers/user_counters_controller.rb index 15ea56e23..0949a233a 100644 --- a/app/controllers/user_counters_controller.rb +++ b/app/controllers/user_counters_controller.rb @@ -3,10 +3,12 @@ class UserCountersController < WithUserController def update counter = current_user.user_counters.where(counter_name: counter_name).first_or_initialize + counter.decay increment = permitted_params[:increment].to_f counter.count += increment - counter.decay counter.decayed_count += increment + counter.save! if counter.new_record? # need to save before applying values if new user counter + apply_values(counter, permitted_params[:values]) if permitted_params[:values] counter.save! render json: counter, status: :ok end @@ -21,7 +23,7 @@ def index protected def permitted_params - params.require(:data).require(:attributes).permit(:increment) + params.require(:data).require(:attributes).permit(:increment, values: []) end def counter_name @@ -29,4 +31,15 @@ def counter_name counter_name += ".#{params[:format]}" if params[:format].present? counter_name end + + # todo: move to model + def apply_values(counter, values) + values_before = counter.values + new_values = values - values_before + new_values.each do |value| + counter.user_counter_values.create!(value: value) + end + counter.count += new_values.count + counter.decayed_count += new_values.count + end end diff --git a/app/models/user_counter.rb b/app/models/user_counter.rb index 4cc6810be..d76398aa4 100644 --- a/app/models/user_counter.rb +++ b/app/models/user_counter.rb @@ -4,11 +4,12 @@ class UserCounter < ApplicationRecord DECAY_RATE = -0.0077 belongs_to :user + has_many :user_counter_values validates :counter_name, format: {with: /\A[-_.a-zA-Z0-9]+\z/, message: "has invalid characters"} def decay - date_now = Date.today - self.last_decay ||= Date.today + date_now = Time.now.utc.to_date + self.last_decay ||= date_now days_to_decay = date_now - last_decay if days_to_decay > 0 @@ -16,4 +17,8 @@ def decay self.last_decay = date_now end end + + def values + user_counter_values.pluck(:value) + end end diff --git a/app/models/user_counter_value.rb b/app/models/user_counter_value.rb new file mode 100644 index 000000000..c2a44d1b9 --- /dev/null +++ b/app/models/user_counter_value.rb @@ -0,0 +1,3 @@ +class UserCounterValue < ApplicationRecord + belongs_to :user_counter +end diff --git a/app/serializers/user_counter_serializer.rb b/app/serializers/user_counter_serializer.rb index 46fde7d45..c22626686 100644 --- a/app/serializers/user_counter_serializer.rb +++ b/app/serializers/user_counter_serializer.rb @@ -3,8 +3,13 @@ class UserCounterSerializer < ActiveModel::Serializer type "user-counter" attributes :id, :count, :decayed_count, :last_decay + attribute :values, if: :has_values? def id object.counter_name end + + def has_values? + object.values.any? + end end diff --git a/app/serializers/user_counter_value_serializer.rb b/app/serializers/user_counter_value_serializer.rb new file mode 100644 index 000000000..f8032095a --- /dev/null +++ b/app/serializers/user_counter_value_serializer.rb @@ -0,0 +1,3 @@ +class UserCounterValueSerializer < ActiveModel::Serializer + attribute :value +end diff --git a/db/migrate/20211110153757_create_user_counters.rb b/db/migrate/20211110153757_create_user_counters.rb index 05fd399e0..34fe4c283 100644 --- a/db/migrate/20211110153757_create_user_counters.rb +++ b/db/migrate/20211110153757_create_user_counters.rb @@ -5,7 +5,7 @@ def change t.string :counter_name t.integer :count, default: 0 t.float :decayed_count, default: 0 - t.date :last_decay, default: -> { "NOW()" } + t.date :last_decay, default: -> { timezone('utc', NOW()) } t.timestamps end diff --git a/db/migrate/20211201161437_create_user_counter_values.rb b/db/migrate/20211201161437_create_user_counter_values.rb new file mode 100644 index 000000000..5efd074af --- /dev/null +++ b/db/migrate/20211201161437_create_user_counter_values.rb @@ -0,0 +1,11 @@ +class CreateUserCounterValues < ActiveRecord::Migration[6.0] + def change + create_table :user_counter_values do |t| + t.string :user_id + t.string :user_counter_id + t.string :value + + t.timestamps + end + end +end diff --git a/db/schema.rb b/db/schema.rb index 2184e9975..b3fd93580 100644 --- a/db/schema.rb +++ b/db/schema.rb @@ -10,7 +10,7 @@ # # It's strongly recommended that you check this file into your version control system. -ActiveRecord::Schema.define(version: 2021_11_10_153757) do +ActiveRecord::Schema.define(version: 2021_12_01_161437) do # These are extensions that must be enabled in order to support this database enable_extension "citext" @@ -226,12 +226,20 @@ t.index ["resource_id"], name: "index_translations_on_resource_id" end + create_table "user_counter_values", force: :cascade do |t| + t.string "user_id" + t.string "user_counter_id" + t.string "value" + t.datetime "created_at", precision: 6, null: false + t.datetime "updated_at", precision: 6, null: false + end + create_table "user_counters", force: :cascade do |t| t.integer "user_id" t.string "counter_name" t.integer "count", default: 0 t.float "decayed_count", default: 0.0 - t.date "last_decay", default: -> { "now()" } + t.date "last_decay", default: -> { "timezone('utc', NOW())" } t.datetime "created_at", precision: 6, null: false t.datetime "updated_at", precision: 6, null: false t.index ["user_id", "counter_name"], name: "index_user_counters_on_user_id_and_counter_name", unique: true diff --git a/spec/acceptance/user_counters_controller_spec.rb b/spec/acceptance/user_counters_controller_spec.rb index 4a949d4d7..d25950abf 100644 --- a/spec/acceptance/user_counters_controller_spec.rb +++ b/spec/acceptance/user_counters_controller_spec.rb @@ -57,12 +57,56 @@ expect(UserCounter.last.last_decay).to eq(Date.today) end end + + context "with values" do + it "create user_counter" do + expect { + do_request data: {type: "user_counter", attributes: {increment: 20, values: ["v1", "v2"]}} + }.to change { UserCounter.count }.by(1) + + expect(status).to eq(200) + json_response = JSON.parse(response_body)["data"] + expect(json_response).not_to be_nil + expect(json_response["id"]).to eq("tool_opens.kgp") + expect(json_response["attributes"]["count"]).to eq(22) + expect(json_response["attributes"]["decayed-count"]).to eq(22) + expect(json_response["attributes"]["last-decay"]).to eq(Date.today.to_s) + expect(UserCounter.last.counter_name).to eq("tool_opens.kgp") + expect(UserCounter.last.count).to eq(22) + expect(UserCounter.last.decayed_count).to eq(22) + expect(UserCounter.last.last_decay).to eq(Date.today) + end + + context "when user_counter exists" do + let!(:user_counter) { FactoryBot.create(:user_counter, user: user, counter_name: "tool_opens.kgp", count: 50, decayed_count: 50, last_decay: 90.days.ago) } + + it "updates the count and decay" do + expect { + do_request data: {type: "user_counter", attributes: {increment: 20, values: ["v1", "v2"]}} + }.to_not change { user_counter.count } + + expect(status).to eq(200) + json_response = JSON.parse(response_body)["data"] + expect(json_response).not_to be_nil + expect(json_response["id"]).to eq("tool_opens.kgp") + expect(json_response["attributes"]["count"]).to eq(72) + expect((json_response["attributes"]["decayed-count"] - 47).abs).to be <= 0.004 # look within 0.004, close enough + expect(json_response["attributes"]["last-decay"]).to eq(Date.today.to_s) + expect(json_response["attributes"]["values"]).to eq(["v1", "v2"]) + expect(UserCounter.last.count).to eq(72) + # get close to 45 -- original value of 50 should decay to 25 with the 90 day half-life, then +20 from the patch count incremement + expect((UserCounter.last.decayed_count - 47).abs).to be <= 0.004 + expect(UserCounter.last.last_decay).to eq(Date.today) + expect(UserCounter.last.values).to eq(["v1", "v2"]) + end + end + end end get "user/counters" do let(:user) { FactoryBot.create(:user) } - let!(:user_counter) { FactoryBot.create(:user_counter, user: user, counter_name: "tool_opens.kgp", count: 50, decayed_count: 50, last_decay: 90.days.ago) } - let!(:user_counter2) { FactoryBot.create(:user_counter, user: user, counter_name: "other.kgp", count: 60, decayed_count: 40, last_decay: 90.days.ago) } + let!(:user_counter) { FactoryBot.create(:user_counter, user: user, counter_name: "tool_opens.kgp", count: 50, decayed_count: 50, last_decay: 90.days.ago.utc) } + let!(:user_counter2) { FactoryBot.create(:user_counter, user: user, counter_name: "other.kgp", count: 60, decayed_count: 40, last_decay: 90.days.ago.utc) } requires_okta_login it "gets counts" do @@ -73,5 +117,19 @@ expected_result = %({"data":[{"id":"tool_opens.kgp","type":"user-counter","attributes":{"count":50,"decayed-count":25.00367978478838,"last-decay":"#{today}"}},{"id":"other.kgp","type":"user-counter","attributes":{"count":60,"decayed-count":20.002943827830705,"last-decay":"#{today}"}}]}) expect(response_body).to eq(expected_result) end + + context "with values" do + let!(:user_value1) { FactoryBot.create(:user_counter_value, user_counter: user_counter, value: "v1") } + let!(:user_value2) { FactoryBot.create(:user_counter_value, user_counter: user_counter, value: "v2") } + + it "includes values" do + do_request + + expect(status).to eq(200) + today = Date.today.to_s + expected_result = %({"data":[{"id":"tool_opens.kgp","type":"user-counter","attributes":{"count":50,"decayed-count":25.00367978478838,"last-decay":"#{today}","values":["v1","v2"]}},{"id":"other.kgp","type":"user-counter","attributes":{"count":60,"decayed-count":20.002943827830705,"last-decay":"#{today}"}}]}) + expect(response_body).to eq(expected_result) + end + end end end diff --git a/spec/factories/user_counter_values.rb b/spec/factories/user_counter_values.rb new file mode 100644 index 000000000..4ccbf0d45 --- /dev/null +++ b/spec/factories/user_counter_values.rb @@ -0,0 +1,5 @@ +FactoryBot.define do + factory :user_counter_value do + value { "value" } + end +end From bf2e63dfb59725550d2fb1da8b68b03fd4a50599 Mon Sep 17 00:00:00 2001 From: Andrew Roth Date: Tue, 14 Dec 2021 11:19:59 -0500 Subject: [PATCH 05/15] remove values from return --- app/serializers/user_counter_serializer.rb | 5 ----- db/migrate/20211110153757_create_user_counters.rb | 2 +- spec/acceptance/user_counters_controller_spec.rb | 3 +-- 3 files changed, 2 insertions(+), 8 deletions(-) diff --git a/app/serializers/user_counter_serializer.rb b/app/serializers/user_counter_serializer.rb index c22626686..46fde7d45 100644 --- a/app/serializers/user_counter_serializer.rb +++ b/app/serializers/user_counter_serializer.rb @@ -3,13 +3,8 @@ class UserCounterSerializer < ActiveModel::Serializer type "user-counter" attributes :id, :count, :decayed_count, :last_decay - attribute :values, if: :has_values? def id object.counter_name end - - def has_values? - object.values.any? - end end diff --git a/db/migrate/20211110153757_create_user_counters.rb b/db/migrate/20211110153757_create_user_counters.rb index 34fe4c283..f8f2e23cd 100644 --- a/db/migrate/20211110153757_create_user_counters.rb +++ b/db/migrate/20211110153757_create_user_counters.rb @@ -5,7 +5,7 @@ def change t.string :counter_name t.integer :count, default: 0 t.float :decayed_count, default: 0 - t.date :last_decay, default: -> { timezone('utc', NOW()) } + t.date :last_decay, default: -> { timezone("utc", NOW()) } t.timestamps end diff --git a/spec/acceptance/user_counters_controller_spec.rb b/spec/acceptance/user_counters_controller_spec.rb index d25950abf..7262f9538 100644 --- a/spec/acceptance/user_counters_controller_spec.rb +++ b/spec/acceptance/user_counters_controller_spec.rb @@ -92,7 +92,6 @@ expect(json_response["attributes"]["count"]).to eq(72) expect((json_response["attributes"]["decayed-count"] - 47).abs).to be <= 0.004 # look within 0.004, close enough expect(json_response["attributes"]["last-decay"]).to eq(Date.today.to_s) - expect(json_response["attributes"]["values"]).to eq(["v1", "v2"]) expect(UserCounter.last.count).to eq(72) # get close to 45 -- original value of 50 should decay to 25 with the 90 day half-life, then +20 from the patch count incremement expect((UserCounter.last.decayed_count - 47).abs).to be <= 0.004 @@ -127,7 +126,7 @@ expect(status).to eq(200) today = Date.today.to_s - expected_result = %({"data":[{"id":"tool_opens.kgp","type":"user-counter","attributes":{"count":50,"decayed-count":25.00367978478838,"last-decay":"#{today}","values":["v1","v2"]}},{"id":"other.kgp","type":"user-counter","attributes":{"count":60,"decayed-count":20.002943827830705,"last-decay":"#{today}"}}]}) + expected_result = %({"data":[{"id":"tool_opens.kgp","type":"user-counter","attributes":{"count":50,"decayed-count":25.00367978478838,"last-decay":"#{today}"}},{"id":"other.kgp","type":"user-counter","attributes":{"count":60,"decayed-count":20.002943827830705,"last-decay":"#{today}"}}]}) expect(response_body).to eq(expected_result) end end From fba04d84bf448ade390febf92cd32c9bbbf85f4d Mon Sep 17 00:00:00 2001 From: Andrew Roth Date: Tue, 14 Dec 2021 13:06:46 -0500 Subject: [PATCH 06/15] switch to integer foreign keys --- db/migrate/20211201161437_create_user_counter_values.rb | 4 ++-- db/schema.rb | 4 ++-- 2 files changed, 4 insertions(+), 4 deletions(-) diff --git a/db/migrate/20211201161437_create_user_counter_values.rb b/db/migrate/20211201161437_create_user_counter_values.rb index 5efd074af..392a44ed1 100644 --- a/db/migrate/20211201161437_create_user_counter_values.rb +++ b/db/migrate/20211201161437_create_user_counter_values.rb @@ -1,8 +1,8 @@ class CreateUserCounterValues < ActiveRecord::Migration[6.0] def change create_table :user_counter_values do |t| - t.string :user_id - t.string :user_counter_id + t.integer :user_id + t.integer :user_counter_id t.string :value t.timestamps diff --git a/db/schema.rb b/db/schema.rb index b3fd93580..cfb47fbbf 100644 --- a/db/schema.rb +++ b/db/schema.rb @@ -227,8 +227,8 @@ end create_table "user_counter_values", force: :cascade do |t| - t.string "user_id" - t.string "user_counter_id" + t.integer "user_id" + t.integer "user_counter_id" t.string "value" t.datetime "created_at", precision: 6, null: false t.datetime "updated_at", precision: 6, null: false From 2130ba2dc07ec5de544ef0e9db5e9b0132ab5307 Mon Sep 17 00:00:00 2001 From: Andrew Roth Date: Wed, 4 May 2022 16:22:12 -0400 Subject: [PATCH 07/15] move apply_values to model --- app/controllers/user_counters_controller.rb | 13 +------------ app/models/user_counter.rb | 10 ++++++++++ 2 files changed, 11 insertions(+), 12 deletions(-) diff --git a/app/controllers/user_counters_controller.rb b/app/controllers/user_counters_controller.rb index 0949a233a..a3b4f81df 100644 --- a/app/controllers/user_counters_controller.rb +++ b/app/controllers/user_counters_controller.rb @@ -8,7 +8,7 @@ def update counter.count += increment counter.decayed_count += increment counter.save! if counter.new_record? # need to save before applying values if new user counter - apply_values(counter, permitted_params[:values]) if permitted_params[:values] + counter.apply_values(permitted_params[:values]) if permitted_params[:values] counter.save! render json: counter, status: :ok end @@ -31,15 +31,4 @@ def counter_name counter_name += ".#{params[:format]}" if params[:format].present? counter_name end - - # todo: move to model - def apply_values(counter, values) - values_before = counter.values - new_values = values - values_before - new_values.each do |value| - counter.user_counter_values.create!(value: value) - end - counter.count += new_values.count - counter.decayed_count += new_values.count - end end diff --git a/app/models/user_counter.rb b/app/models/user_counter.rb index d76398aa4..f62a60d25 100644 --- a/app/models/user_counter.rb +++ b/app/models/user_counter.rb @@ -21,4 +21,14 @@ def decay def values user_counter_values.pluck(:value) end + + def apply_values(values) + values_before = self.values + new_values = values - values_before + new_values.each do |value| + user_counter_values.create!(value: value) + end + self.count += new_values.count + self.decayed_count += new_values.count + end end From 6e47ca1cfafe4bedb983bde5b8bc400c366437e9 Mon Sep 17 00:00:00 2001 From: Andrew Roth Date: Wed, 4 May 2022 17:14:10 -0400 Subject: [PATCH 08/15] use an array for values --- app/controllers/user_counters_controller.rb | 2 +- app/models/user_counter.rb | 12 +----- ...211201161437_create_user_counter_values.rb | 11 ----- ...20504203747_add_values_to_user_counters.rb | 8 ++++ ...204229_remove_user_counter_values_table.rb | 6 +++ db/schema.rb | 40 ++++++------------- .../user_counters_controller_spec.rb | 8 ++-- 7 files changed, 34 insertions(+), 53 deletions(-) delete mode 100644 db/migrate/20211201161437_create_user_counter_values.rb create mode 100644 db/migrate/20220504203747_add_values_to_user_counters.rb create mode 100644 db/migrate/20220504204229_remove_user_counter_values_table.rb diff --git a/app/controllers/user_counters_controller.rb b/app/controllers/user_counters_controller.rb index a3b4f81df..28203c9b3 100644 --- a/app/controllers/user_counters_controller.rb +++ b/app/controllers/user_counters_controller.rb @@ -14,7 +14,7 @@ def update end def index - counters = current_user.user_counters + counters = current_user.user_counters.order(:id) # decay but don't save as it's not needed and it'll be more efficient this way -- the in-memory decay calculation is very fast counters.each(&:decay) render json: counters, status: :ok diff --git a/app/models/user_counter.rb b/app/models/user_counter.rb index f62a60d25..5242f9967 100644 --- a/app/models/user_counter.rb +++ b/app/models/user_counter.rb @@ -4,7 +4,6 @@ class UserCounter < ApplicationRecord DECAY_RATE = -0.0077 belongs_to :user - has_many :user_counter_values validates :counter_name, format: {with: /\A[-_.a-zA-Z0-9]+\z/, message: "has invalid characters"} def decay @@ -18,16 +17,9 @@ def decay end end - def values - user_counter_values.pluck(:value) - end - def apply_values(values) - values_before = self.values - new_values = values - values_before - new_values.each do |value| - user_counter_values.create!(value: value) - end + new_values = values - self.values + self.values += new_values self.count += new_values.count self.decayed_count += new_values.count end diff --git a/db/migrate/20211201161437_create_user_counter_values.rb b/db/migrate/20211201161437_create_user_counter_values.rb deleted file mode 100644 index 392a44ed1..000000000 --- a/db/migrate/20211201161437_create_user_counter_values.rb +++ /dev/null @@ -1,11 +0,0 @@ -class CreateUserCounterValues < ActiveRecord::Migration[6.0] - def change - create_table :user_counter_values do |t| - t.integer :user_id - t.integer :user_counter_id - t.string :value - - t.timestamps - end - end -end diff --git a/db/migrate/20220504203747_add_values_to_user_counters.rb b/db/migrate/20220504203747_add_values_to_user_counters.rb new file mode 100644 index 000000000..0ec50fe86 --- /dev/null +++ b/db/migrate/20220504203747_add_values_to_user_counters.rb @@ -0,0 +1,8 @@ +class AddValuesToUserCounters < ActiveRecord::Migration[6.1] + def change + change_table :user_counters do |t| + t.string 'values', array: true, default: "{}" + end + add_index :user_counters, :values, using: 'gin' + end +end diff --git a/db/migrate/20220504204229_remove_user_counter_values_table.rb b/db/migrate/20220504204229_remove_user_counter_values_table.rb new file mode 100644 index 000000000..f6247293a --- /dev/null +++ b/db/migrate/20220504204229_remove_user_counter_values_table.rb @@ -0,0 +1,6 @@ +class RemoveUserCounterValuesTable < ActiveRecord::Migration[6.1] + def change + return unless ActiveRecord::Base.connection.table_exists?("user_counter_values") + drop_table :user_counter_values + end +end diff --git a/db/schema.rb b/db/schema.rb index 0e1b0c05a..ba81b58ea 100644 --- a/db/schema.rb +++ b/db/schema.rb @@ -10,7 +10,7 @@ # # It's strongly recommended that you check this file into your version control system. -ActiveRecord::Schema.define(version: 2022_03_25_184941) do +ActiveRecord::Schema.define(version: 2022_05_04_204229) do # These are extensions that must be enabled in order to support this database enable_extension "citext" @@ -233,12 +233,15 @@ t.index ["resource_id"], name: "index_translations_on_resource_id" end - create_table "user_counter_values", force: :cascade do |t| - t.integer "user_id" - t.integer "user_counter_id" - t.string "value" - t.datetime "created_at", precision: 6, null: false - t.datetime "updated_at", precision: 6, null: false + create_table "translations_bkup", id: false, force: :cascade do |t| + t.integer "id" + t.boolean "is_published" + t.integer "version" + t.integer "resource_id" + t.integer "language_id" + t.string "translated_name" + t.string "translated_description" + t.string "manifest_name" end create_table "user_counters", force: :cascade do |t| @@ -246,10 +249,11 @@ t.string "counter_name" t.integer "count", default: 0 t.float "decayed_count", default: 0.0 - t.date "last_decay", default: -> { "timezone('utc', NOW())" } + t.date "last_decay" t.datetime "created_at", precision: 6, null: false t.datetime "updated_at", precision: 6, null: false - t.index ["user_id", "counter_name"], name: "index_user_counters_on_user_id_and_counter_name", unique: true + t.string "values", default: [], array: true + t.index ["values"], name: "index_user_counters_on_values", using: :gin end create_table "users", force: :cascade do |t| @@ -266,22 +270,4 @@ add_foreign_key "active_storage_attachments", "active_storage_blobs", column: "blob_id" add_foreign_key "active_storage_variant_records", "active_storage_blobs", column: "blob_id" - add_foreign_key "attachments", "resources" - add_foreign_key "attributes", "resources" - add_foreign_key "auth_tokens", "access_codes" - add_foreign_key "custom_manifests", "languages" - add_foreign_key "custom_manifests", "resources" - add_foreign_key "custom_pages", "languages" - add_foreign_key "custom_pages", "pages" - add_foreign_key "follow_ups", "destinations" - add_foreign_key "follow_ups", "languages" - add_foreign_key "pages", "resources" - add_foreign_key "resources", "resource_types" - add_foreign_key "resources", "systems" - add_foreign_key "translated_attributes", "attributes" - add_foreign_key "translated_attributes", "translations" - add_foreign_key "translated_pages", "languages" - add_foreign_key "translated_pages", "resources" - add_foreign_key "translations", "languages" - add_foreign_key "translations", "resources" end diff --git a/spec/acceptance/user_counters_controller_spec.rb b/spec/acceptance/user_counters_controller_spec.rb index 7262f9538..a0c5069db 100644 --- a/spec/acceptance/user_counters_controller_spec.rb +++ b/spec/acceptance/user_counters_controller_spec.rb @@ -59,7 +59,7 @@ end context "with values" do - it "create user_counter" do + it "creates user_counter" do expect { do_request data: {type: "user_counter", attributes: {increment: 20, values: ["v1", "v2"]}} }.to change { UserCounter.count }.by(1) @@ -118,15 +118,15 @@ end context "with values" do - let!(:user_value1) { FactoryBot.create(:user_counter_value, user_counter: user_counter, value: "v1") } - let!(:user_value2) { FactoryBot.create(:user_counter_value, user_counter: user_counter, value: "v2") } - it "includes values" do + user_counter.update(values: ["v1", "v2"]) do_request expect(status).to eq(200) today = Date.today.to_s expected_result = %({"data":[{"id":"tool_opens.kgp","type":"user-counter","attributes":{"count":50,"decayed-count":25.00367978478838,"last-decay":"#{today}"}},{"id":"other.kgp","type":"user-counter","attributes":{"count":60,"decayed-count":20.002943827830705,"last-decay":"#{today}"}}]}) + puts "expected: #{expected_result}" + puts "got: #{response_body}" expect(response_body).to eq(expected_result) end end From c870e2730b92f39fefd62ba39a6f35337e150894 Mon Sep 17 00:00:00 2001 From: Andrew Roth Date: Wed, 4 May 2022 17:42:00 -0400 Subject: [PATCH 09/15] remove factory for user_counter_values --- spec/factories/user_counter_values.rb | 5 ----- 1 file changed, 5 deletions(-) delete mode 100644 spec/factories/user_counter_values.rb diff --git a/spec/factories/user_counter_values.rb b/spec/factories/user_counter_values.rb deleted file mode 100644 index 4ccbf0d45..000000000 --- a/spec/factories/user_counter_values.rb +++ /dev/null @@ -1,5 +0,0 @@ -FactoryBot.define do - factory :user_counter_value do - value { "value" } - end -end From 802f4ce9c890855edaebdb0929cba0b8d9ca7754 Mon Sep 17 00:00:00 2001 From: Andrew Roth Date: Wed, 4 May 2022 17:50:21 -0400 Subject: [PATCH 10/15] standardrb fixes --- db/migrate/20220504203747_add_values_to_user_counters.rb | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/db/migrate/20220504203747_add_values_to_user_counters.rb b/db/migrate/20220504203747_add_values_to_user_counters.rb index 0ec50fe86..cd0cf22fe 100644 --- a/db/migrate/20220504203747_add_values_to_user_counters.rb +++ b/db/migrate/20220504203747_add_values_to_user_counters.rb @@ -1,8 +1,8 @@ class AddValuesToUserCounters < ActiveRecord::Migration[6.1] def change change_table :user_counters do |t| - t.string 'values', array: true, default: "{}" + t.string "values", array: true, default: "{}" end - add_index :user_counters, :values, using: 'gin' + add_index :user_counters, :values, using: "gin" end end From f747bfa90df985ca8fedacfaa00eaca3eebd844c Mon Sep 17 00:00:00 2001 From: Andrew Roth Date: Wed, 4 May 2022 18:03:39 -0400 Subject: [PATCH 11/15] undo schema changes that should not have been in --- db/schema.rb | 13 +------------ 1 file changed, 1 insertion(+), 12 deletions(-) diff --git a/db/schema.rb b/db/schema.rb index ba81b58ea..fcfdef8d3 100644 --- a/db/schema.rb +++ b/db/schema.rb @@ -233,23 +233,12 @@ t.index ["resource_id"], name: "index_translations_on_resource_id" end - create_table "translations_bkup", id: false, force: :cascade do |t| - t.integer "id" - t.boolean "is_published" - t.integer "version" - t.integer "resource_id" - t.integer "language_id" - t.string "translated_name" - t.string "translated_description" - t.string "manifest_name" - end - create_table "user_counters", force: :cascade do |t| t.integer "user_id" t.string "counter_name" t.integer "count", default: 0 t.float "decayed_count", default: 0.0 - t.date "last_decay" + t.date "last_decay", default: -> { "timezone('utc', NOW())" } t.datetime "created_at", precision: 6, null: false t.datetime "updated_at", precision: 6, null: false t.string "values", default: [], array: true From 7dbb33f0a095d5d3740d84efd283a1683cd5494f Mon Sep 17 00:00:00 2001 From: Andrew Roth Date: Wed, 4 May 2022 18:05:09 -0400 Subject: [PATCH 12/15] remove debugging code --- spec/acceptance/user_counters_controller_spec.rb | 2 -- 1 file changed, 2 deletions(-) diff --git a/spec/acceptance/user_counters_controller_spec.rb b/spec/acceptance/user_counters_controller_spec.rb index a0c5069db..1aaebf643 100644 --- a/spec/acceptance/user_counters_controller_spec.rb +++ b/spec/acceptance/user_counters_controller_spec.rb @@ -125,8 +125,6 @@ expect(status).to eq(200) today = Date.today.to_s expected_result = %({"data":[{"id":"tool_opens.kgp","type":"user-counter","attributes":{"count":50,"decayed-count":25.00367978478838,"last-decay":"#{today}"}},{"id":"other.kgp","type":"user-counter","attributes":{"count":60,"decayed-count":20.002943827830705,"last-decay":"#{today}"}}]}) - puts "expected: #{expected_result}" - puts "got: #{response_body}" expect(response_body).to eq(expected_result) end end From 4273586bb65cbac2e46c6492c6cc9db44c71f3bf Mon Sep 17 00:00:00 2001 From: Andrew Roth Date: Tue, 10 May 2022 15:03:17 -0400 Subject: [PATCH 13/15] remove unneeded user counter values files --- app/models/user_counter_value.rb | 3 --- app/serializers/user_counter_value_serializer.rb | 3 --- 2 files changed, 6 deletions(-) delete mode 100644 app/models/user_counter_value.rb delete mode 100644 app/serializers/user_counter_value_serializer.rb diff --git a/app/models/user_counter_value.rb b/app/models/user_counter_value.rb deleted file mode 100644 index c2a44d1b9..000000000 --- a/app/models/user_counter_value.rb +++ /dev/null @@ -1,3 +0,0 @@ -class UserCounterValue < ApplicationRecord - belongs_to :user_counter -end diff --git a/app/serializers/user_counter_value_serializer.rb b/app/serializers/user_counter_value_serializer.rb deleted file mode 100644 index f8032095a..000000000 --- a/app/serializers/user_counter_value_serializer.rb +++ /dev/null @@ -1,3 +0,0 @@ -class UserCounterValueSerializer < ActiveModel::Serializer - attribute :value -end From 9ef690829de0e6efe3b4fb2934ac43b9785020a7 Mon Sep 17 00:00:00 2001 From: Andrew Roth Date: Tue, 10 May 2022 15:13:10 -0400 Subject: [PATCH 14/15] add meta dtd --- public/xmlns/meta.xsd | 0 1 file changed, 0 insertions(+), 0 deletions(-) create mode 100644 public/xmlns/meta.xsd diff --git a/public/xmlns/meta.xsd b/public/xmlns/meta.xsd new file mode 100644 index 000000000..e69de29bb From 495707fc2a8fe9b067232fefedbb0d7c536f4dc3 Mon Sep 17 00:00:00 2001 From: Andrew Roth Date: Wed, 11 May 2022 12:00:27 -0400 Subject: [PATCH 15/15] add back indexes --- db/schema.rb | 18 ++++++++++++++++++ 1 file changed, 18 insertions(+) diff --git a/db/schema.rb b/db/schema.rb index fcfdef8d3..08f4727b8 100644 --- a/db/schema.rb +++ b/db/schema.rb @@ -259,4 +259,22 @@ add_foreign_key "active_storage_attachments", "active_storage_blobs", column: "blob_id" add_foreign_key "active_storage_variant_records", "active_storage_blobs", column: "blob_id" + add_foreign_key "attachments", "resources" + add_foreign_key "attributes", "resources" + add_foreign_key "auth_tokens", "access_codes" + add_foreign_key "custom_manifests", "languages" + add_foreign_key "custom_manifests", "resources" + add_foreign_key "custom_pages", "languages" + add_foreign_key "custom_pages", "pages" + add_foreign_key "follow_ups", "destinations" + add_foreign_key "follow_ups", "languages" + add_foreign_key "pages", "resources" + add_foreign_key "resources", "resource_types" + add_foreign_key "resources", "systems" + add_foreign_key "translated_attributes", "attributes" + add_foreign_key "translated_attributes", "translations" + add_foreign_key "translated_pages", "languages" + add_foreign_key "translated_pages", "resources" + add_foreign_key "translations", "languages" + add_foreign_key "translations", "resources" end