Merge pull request #198 from openstax/fix_sync

Fix 500 errors on sync API calls

Author: Dantemss

app/controllers/api/v1/application_groups_controller.rb

@@ -54,11 +54,11 @@ def updates
 
     After you called the API and received your response, the group had a member added, setting unread_updates to 2.
 
-    `application_groups = {id: 13, read_updates: 1}` – will decrease unread_updates by 1, setting it to 1. The group will be sent again the next time you call `updates`, so you won't miss the updated information.
+    `application_groups = {id: 13, read_updates: 1}` – will not affect the record. The group will be sent again the next time you call `updates`, so you won't miss the updated information.
   EOS
   def updated
     OSU::AccessPolicy.require_action_allowed!(:updated, current_api_user, ApplicationGroup)
-    errors = MarkApplicationGroupsUpdatesAsRead.call(current_application,
+    errors = MarkApplicationGroupUpdatesAsRead.call(current_application,
                ActiveSupport::JSON.decode(request.body)).errors
     head (errors.any? ? :internal_server_error : :no_content)
   end

app/controllers/api/v1/application_users_controller.rb

@@ -196,11 +196,11 @@ def updates
 
     After you called the API and received your response, the user updated their profile in Accounts, setting unread_updates to 2.
 
-    `application_users = {id: 13, read_updates: 1}` – will decrease unread_updates by 1, setting it to 1. The user will be sent again the next time you call `updates`, so you won't miss the updated information.
+    `application_users = {id: 13, read_updates: 1}` – will not affect the record. The user will be sent again the next time you call `updates`, so you won't miss the updated information.
   EOS
   def updated
     OSU::AccessPolicy.require_action_allowed!(:updated, current_api_user, ApplicationUser)
-    errors = MarkApplicationUsersUpdatesAsRead.call(current_application,
+    errors = MarkApplicationUserUpdatesAsRead.call(current_application,
                ActiveSupport::JSON.decode(request.body)).errors
     head (errors.any? ? :internal_server_error : :no_content)
   end

app/controllers/users_controller.rb

@@ -2,8 +2,7 @@ class UsersController < ApplicationController
 
   skip_before_filter :registration, only: [:register]
 
-  fine_print_skip :general_terms_of_use, :privacy_policy,
-                  only: [:register]
+  fine_print_skip :general_terms_of_use, :privacy_policy, only: [:register]
 
   def edit
     OSU::AccessPolicy.require_action_allowed!(:update, current_user, current_user)
@@ -12,7 +11,7 @@ def edit
   def update
     OSU::AccessPolicy.require_action_allowed!(:update, current_user, current_user)
     if current_user.update_attributes(user_params)
-      redirect_to profile_path, notice: 'Profile updated'
+      redirect_to profile_path, notice: 'Your profile has been updated. These changes may take a few minutes to propagate to the entire site.'
     else
       flash.now[:alert] ||= []
       current_user.errors.full_messages.each do |msg|

app/routines/mark_application_group_updates_as_read.rb

@@ -0,0 +1,27 @@
+# Routine for marking all given application groups' updates as read
+#
+# Caller provides the current application, plus an array of hashes with the following
+# 2 required keys: 'id' contains the ApplicationGroup's ID and
+# read_updates contains the last received value of unread_updates.
+
+class MarkApplicationGroupUpdatesAsRead
+
+  # This transaction needs :repeatable_read to prevent missed updates
+  lev_routine transaction: :repeatable_read
+
+  protected
+
+  def exec(application, application_group_hashes)
+    return if application.blank? || application_group_hashes.blank?
+
+    application.application_groups.where do
+      cumulative_query = nil
+      application_group_hashes.each do |hash|
+        query = (group_id == hash['group_id']) & (unread_updates == hash['read_updates'])
+        cumulative_query = cumulative_query.nil? ? query : cumulative_query | query
+      end
+      cumulative_query
+    end.update_all("unread_updates = 0")
+  end
+
+end

app/routines/mark_application_groups_updates_as_read.rb

@@ -0,0 +1,27 @@
+# Routine for marking all given application users' updates as read
+#
+# Caller provides the current application, plus an array of hashes with the following
+# 2 required keys: 'id' contains the ApplicationUser's ID and
+# read_updates contains the last received value of unread_updates.
+
+class MarkApplicationUserUpdatesAsRead
+
+  # This transaction needs :repeatable_read to prevent missed updates
+  lev_routine transaction: :repeatable_read
+
+  protected
+
+  def exec(application, application_user_hashes)
+    return if application.blank? || application_user_hashes.blank?
+
+    application.application_users.where do
+      cumulative_query = nil
+      application_user_hashes.each do |hash|
+        query = (user_id == hash['user_id']) & (unread_updates == hash['read_updates'])
+        cumulative_query = cumulative_query.nil? ? query : cumulative_query | query
+      end
+      cumulative_query
+    end.update_all("unread_updates = 0")
+  end
+
+end

app/routines/mark_application_user_updates_as_read.rb

@@ -203,21 +203,21 @@
       expect(application_group_1.reload.unread_updates).to eq 4
 
       api_put :updated, untrusted_application_token, raw_post_data: [
-        {id: application_group_1.id, read_updates: 2}].to_json
+        {group_id: group_1.id, read_updates: 2}].to_json
 
       expect(response.status).to eq(204)
 
-      expect(application_group_1.reload.unread_updates).to eq 2
+      expect(application_group_1.reload.unread_updates).to eq 4
 
       api_put :updated, untrusted_application_token, raw_post_data: [
-        {id: application_group_1.id, read_updates: 1}].to_json
+        {group_id: group_1.id, read_updates: 1}].to_json
 
       expect(response.status).to eq(204)
 
-      expect(application_group_1.reload.unread_updates).to eq 1
+      expect(application_group_1.reload.unread_updates).to eq 4
 
       api_put :updated, untrusted_application_token, raw_post_data: [
-        {id: application_group_1.id, read_updates: 2}].to_json
+        {group_id: group_1.id, read_updates: 4}].to_json
 
       expect(response.status).to eq(204)
 

app/routines/mark_application_users_updates_as_read.rb

@@ -61,7 +61,7 @@
       expect(response.body).to eq(expected_response)
     end
 
-    it "raises not found when when not found" do
+    it "raises not found when not found" do
       expect {
         api_get :find_by_username, untrusted_application_token, parameters: { username: 'foo' }
       }.to raise_error(ActiveRecord::RecordNotFound)
@@ -296,21 +296,21 @@
       expect(app_user.reload.unread_updates).to eq 4
 
       api_put :updated, untrusted_application_token, raw_post_data: [
-        {id: app_user.id, read_updates: 2}].to_json
+        {user_id: user_2.id, read_updates: 2}].to_json
 
       expect(response.status).to eq(204)
 
-      expect(app_user.reload.unread_updates).to eq 2
+      expect(app_user.reload.unread_updates).to eq 4
 
       api_put :updated, untrusted_application_token, raw_post_data: [
-        {id: app_user.id, read_updates: 1}].to_json
+        {user_id: user_2.id, read_updates: 1}].to_json
 
       expect(response.status).to eq(204)
 
-      expect(app_user.reload.unread_updates).to eq 1
+      expect(app_user.reload.unread_updates).to eq 4
 
       api_put :updated, untrusted_application_token, raw_post_data: [
-        {id: app_user.id, read_updates: 2}].to_json
+        {user_id: user_2.id, read_updates: 4}].to_json
 
       expect(response.status).to eq(204)