user2759575
user2759575

Reputation: 553

Delay update from unread to read

I have a notifications page where I want to show only the notifications that a user hasn't seen. I'm trying to do this by making each notification :read => false by default but changing it to :read => true when the user clicks on the notification link (the '.bell' class). The problem is it's changed to => true exactly when the user clicks on the link, so when the notification page loads everything has been 'read' and it displays nothing. I'm not sure how to delay this change until the page has been loaded.

In my controller I have this:

def notifications
  @activities = PublicActivity::Activity.order("created_at desc").where(recipient_id: current_user.id)
  @notification_count = @activities.where(:read => false).count  
end

def read_all_notifications
  PublicActivity::Activity.where(recipient_id: current_user.id).update_all(:read => true)
end 

Then in my routes:

get 'users/:id/allnotifications', to: 'users#allnotifications', as: :allnotifications 
post 'users/read_all_notifications', to: 'users#read_all_notifications', as: :read_all_notifications 

and in users.js.coffee:

$(document).on 'click' , '.bell' , (e)->
    $.ajax '/users/read_all_notifications' ,
        type: "post"
        dataType: "json"
        beforeSend: (xhr) ->
          xhr.setRequestHeader "X-CSRF-Token", $("meta[name=\"csrf-token\"]").attr("content")
        cache: false

Edit

I am now putting this at the bottom of my notifications view:

<script>
$(document) ()->
    $.ajax '/users/read_all_notifications' ,
       type: "post"
       dataType: "json"
       beforeSend: (xhr) ->
         xhr.setRequestHeader "X-CSRF-Token", $("meta[name=\"csrf-token\"]").attr("content")
       cache: false  
</script>

Upvotes: 0

Views: 708

Answers (4)

craig.kaminsky
craig.kaminsky

Reputation: 5598

I may be totally under-thinking this but, any reason in your app/logic that you can't just use an after_action in your controller?

Controller

after_action :read_all_notifications 

def notifications
  @activities = PublicActivity::Activity.order("created_at desc").where(recipient_id: current_user.id)
  @notification_count = @activities.where(:read => false).count  
end

def read_all_notifications
  PublicActivity::Activity.where(recipient_id: current_user.id).update_all(:read => true)
end 

In theory, this would mark all unread notifications as read once the controller finished loading (and displaying) the page.

Again, I may well be under-thinking your issue and apologies if I have, but, I did want to offer this possible solution.

Upvotes: 4

vereecjw
vereecjw

Reputation: 139

I may be missing some of the issue here, but i believe you want a result where when they click on the notifications page, it renders all of the unread notifications, keeps the unread number at the current state. If they user goes to another page, they will now see 0 unread notifications (assuming no new notifications have been created) and if they click the link again will see no notifications.

If that is the case, a very simple solution would be

def notifications
  #Get UNREAD activities AND force execution
  @activities = PublicActivity::Activity.order("created_at desc").where(recipient_id: current_user.id).where(:read => false).all
  @notification_count = @activities.where(:read => false).count  
  #Mark items read
  PublicActivity::Activity.where(recipient_id: current_user.id).where(:read => false).update_all(:read => true)
end

Note. The queries changed slightly. the .all should force execution of the query.

Upvotes: 1

Richard Peck
Richard Peck

Reputation: 76774

Attribute

You'll be much better using one of the state_machine gems (state_machine, aasm, workflow)

We use aasm, primarily because state_machine is no longer active.

#app/models/activity.rb
class Activity < ActiveRecord::Base
   include AASM

   aasm do
     state :unread, :initial => true
     state :read

     event :open do
       transitions :from => :unread, :to => :read
     end

     event :mark_as_read do
       transitions :from => :read, :to => :unread
     end
  end
end

This will "over engineer" your requirements, but will certainly provide you with the functionality you need to make it extensible

By including one of these gems, it will automatically populate the state attribute in your table. This is both more verbose & less succinct than using a standard boolean, but what it really helps you do is provide you with object orientated public methods:

job = Job.new
job.sleeping? # => true
job.may_run?  # => true
job.run
job.running?  # => true
job.sleeping? # => false
job.may_run?  # => false
job.run       # => raises AASM::InvalidTransition

In your case, you'll have:

notification = PublicActivity::Activity.new

notification.read? # -> false
notification.unread? #-> true

notification.mark_as_read #-> "unread"
noticiation.open  #-> "read"
notification.sate #-> "unread"

Associations

Secondly, you'll need to use associations & scopes to manage your data.

Any time you call .where on a foreign_key, you're doing the work which ActiveRecord could be doing for you:

#app/models/public_activity/activity.rb
class PublicActivity::Activity < ActiveRecord::Base
   #fields id | user_id | created_at | updated_at

   belongs_to :user
   #belongs_to :recipient, class_name: "User", foreign_key: "recipient_id" #-> received --only if user to user

   scope :recent, -> { order created_at: desc }
   scope :read,   -> { where status: "read" }
end

#app/models/users.rb
class User < ActiveRecord::Base
   has_many :notifications, class_name: "PublicActivity::Activity"
end

This should give you the ability to call the following:

current_user.notifications.recent.read

Transition

so when the notification page loads everything has been 'read' and it displays nothing. I'm not sure how to delay this change until the page has been loaded.

From what I understand of your request, just use a delayed ajax request:

#config/routes.rb
resources :users do
  resources :notifications, only: [:index, :show] do
      post :read_all, on: :collection #-> domain.com/notifications/read_all
  end
end

This will give you the ability to use the following:

#app/controllers/notifications_controller.rb
class NotificationsController < ApplicationController
  # before_action :set_user -> only needed if not using current_user

   def index
      @notifications = current_user.notifications
   end

   private

   def set_user
      @user = User.find params[:user_id]
   end
end

It will give you the ability to use a delayed Ajax request as follows:

#app/assets/javascripts/delayed_ajax.js
jQuery(document).ready(function () {
    setTimeout(update_read, 1000);
});

var update_read = function() {
   $.ajax({
      url: "users/....../read_all",
      success: function(data) {
          alert("Success");
      }
   });
};

The above javascript will have to be called explicitly from the view you wish to use it in (or at least have some identifying value to determine when it fires).

The trick here will be that it uses ajax. The importance of this is that the Ajax will only work after the page has loaded (ensuring your issue is not repeated), and it will only fire after a certain amount of time.

This should work for you

Upvotes: 1

Marcelo Ribeiro
Marcelo Ribeiro

Reputation: 1738

I would remove the listener on the click, and would add it to the page when it is loaded instead. That way, once the page is loaded, the ajax request is triggered.

I assume you have a notifications template file (notifications.html.erb maybe?). That's where you should require your js call to trigger the "mark all as read". Be careful not to keep the js trigger in users.js.coffee as that javascript file is probably loaded on every request and not only when notifications template is loaded.

Upvotes: 0

Related Questions