Expose bounce details via the API to siteadmins

Fixes CNVS-22965

Test plan:
 - Create a user and a communication channel for that user, or reuse
   an existing one that has not been deleted
 - Bounce details:
   - From a console, do the following, where "..." is the id of the
     communication channel you're working with:
       c = CommunicationChannel.find(...)
       c.last_bounce_details =
         {'bouncedRecipients' => [{'diagnosticCode' => 'foo'}]}
       c.save!
   - As a siteadmin, hit /api/v1/users/.../communication_channels, where
     ... is the id of the user that owns the communication channel
   - Verify that the communication channel includes a
     "last_bounce_summary" property whose value is "foo"
 - Transient bounce details:
   - From a console, do the following, where "..." is the id of the
     communication channel you're working with:
       c = CommunicationChannel.find(...)
       c.last_transient_bounce_details =
         {'bouncedRecipients' => [{'diagnosticCode' => 'foo'}]}
       c.save!
   - As a siteadmin, hit /api/v1/users/.../communication_channels, where
     ... is the id of the user that owns the communication channel
   - Verify that the communication channel includes a
     "last_transient_bounce_summary" property whose value is "foo"
 - Bounce date:
   - From a console, do the following, where "..." is the id of the
     communication channel you're working with:
       c = CommunicationChannel.find(...)
       c.last_bounce_at = '2015-01-01T01:01:01.000Z'
       c.save!
   - As a siteadmin, hit /api/v1/users/.../communication_channels, where
     ... is the id of the user that owns the communication channel
   - Verify that the communication channel includes a
     "last_bounce_at" key whose values is "2015-01-01T01:01:01.000Z"
 - Transient bounce date:
  - From a console, do the following, where "..." is the id of the
     communication channel you're working with:
       c = CommunicationChannel.find(...)
       c.last_transient_bounce_at = '2015-01-01T01:01:01.000Z'
       c.save!
   - As a siteadmin, hit /api/v1/users/.../communication_channels, where
     ... is the id of the user that owns the communication channel
   - Verify that the communication channel includes a
     "last_transient_bounce_at" key whose values is "2015-01-01T01:01:01.000Z"
 - Suppression bounce date:
   - From a console, do the following, where "..." is the id of the
     communication channel you're working with:
       c = CommunicationChannel.find(...)
       c.last_suppression_bounce_at = '2015-01-01T01:01:01.000Z'
       c.save!
   - As a siteadmin, hit /api/v1/users/.../communication_channels, where
     ... is the id of the user that owns the communication channel
   - Verify that the communication channel includes a
     "last_suppression_bounce_at" key whose values is "2015-01-01T01:01:01.000Z"
 - Restricted to site admins:
   - As the user whose communication channel you were working with,
     hit /api/v1/users/.../communication_channels, where ... is that
     user's id
   - Verify that the communication channel does not include
     "last_bounce_at", "last_suppression_bounce_at",
     "last_transient_bounce_at", "last_bounce_summary",
     or "last_transient_bounce_summary"

Change-Id: I10ed797620392229a55ffc797b2b35d3e979e19a
Reviewed-on: https://gerrit.instructure.com/62578
Reviewed-by: Andrew Butterfield <abutterfield@instructure.com>
Tested-by: Jenkins
QA-Review: Adrian Russell <arussell@instructure.com>
Product-Review: Allison Weiss <allison@instructure.com>

app/models/communication_channel.rb

@@ -123,6 +123,7 @@ class CommunicationChannel < ActiveRecord::Base
 
     given { |user| Account.site_admin.grants_right?(user, :read_messages) }
     can :reset_bounce_count
+    can :read_bounce_details
   end
 
   def pseudonym
@@ -470,6 +471,14 @@ class CommunicationChannel < ActiveRecord::Base
     end
   end
 
+  def last_bounce_summary
+    last_bounce_details.try(:[], 'bouncedRecipients').try(:[], 0).try(:[], 'diagnosticCode')
+  end
+
+  def last_transient_bounce_summary
+    last_transient_bounce_details.try(:[], 'bouncedRecipients').try(:[], 0).try(:[], 'diagnosticCode')
+  end
+
   def self.find_by_confirmation_code(code)
     where(confirmation_code: code).first
 

lib/api/v1/communication_channel.rb

@@ -19,16 +19,6 @@
 module Api::V1::CommunicationChannel
   include Api::V1::Json
 
-  # Internal: The attributes returned by communication_channel_json.
-  #
-  # Uses the method "path_description" instead of the field "path" because
-  # when path_type is twitter or yo, it goes and fetches tha user's account
-  # name with a fallback display value.
-  JSON_OPTS = {
-    :only => %w{ id path_type position workflow_state user_id },
-    :methods => %w{ path_description }
-  }
-
   # Public: Given a communication channel, return it in an API-friendly format.
   #
   # channel - The communication channel to turn into a hash.
@@ -43,7 +33,26 @@ module Api::V1::CommunicationChannel
   #   :user_id
   #   :workflow_state
   def communication_channel_json(channel, current_user, session)
-    api_json(channel, current_user, session, JSON_OPTS).tap do |json|
+    only = %w{ id path_type position workflow_state user_id }
+    # Uses the method "path_description" instead of the field "path" because
+    # when path_type is twitter or yo, it goes and fetches tha user's account
+    # name with a fallback display value.
+    methods = %w{ path_description }
+
+    # If the user is super special, show them this channel's bounce details
+    if channel.grants_right?(current_user, :read_bounce_details)
+      only += [
+        'last_bounce_at',
+        'last_transient_bounce_at',
+        'last_suppression_bounce_at'
+      ]
+      methods += [
+        'last_bounce_summary',
+        'last_transient_bounce_summary'
+      ]
+    end
+
+    api_json(channel, current_user, session, only: only, methods: methods).tap do |json|
       # Rename attributes for mass-consumption
       json[:address] = json.delete(:path_description)
       json[:type]    = json.delete(:path_type)

spec/apis/v1/communication_channels_api_spec.rb

@@ -32,8 +32,8 @@ describe 'CommunicationChannels API', type: :request do
     end
 
     context 'an authorized user' do
-      it 'should list all channels' do
+      it 'should list all channels without bounce information for a user that is not a site admin' do
-        json = api_call(:get, @path, @path_options)
+        json = api_call_as_user(@someone, :get, @path, @path_options)
 
         cc = @someone.communication_channel
         expect(json).to eql [{
@@ -44,6 +44,24 @@ describe 'CommunicationChannels API', type: :request do
           'workflow_state' => 'unconfirmed',
           'user_id'  => cc.user_id }]
       end
+
+      it 'should list all channels with bounce information for a user that is a site admin' do
+        json = api_call_as_user(@admin, :get, @path, @path_options)
+
+        cc = @someone.communication_channel
+        expect(json).to eql [{
+          'id'       => cc.id,
+          'address'  => cc.path,
+          'type'     => cc.path_type,
+          'position' => cc.position,
+          'workflow_state' => 'unconfirmed',
+          'user_id'  => cc.user_id,
+          'last_bounce_at' => nil,
+          'last_bounce_summary' => nil,
+          'last_suppression_bounce_at' => nil,
+          'last_transient_bounce_at' => nil,
+          'last_transient_bounce_summary' => nil }]
+      end
     end
 
     context 'an unauthorized user' do
@@ -93,7 +111,12 @@ describe 'CommunicationChannels API', type: :request do
         'type' => 'email',
         'workflow_state' => 'active',
         'user_id' => @someone.id,
-        'position' => 2
+        'position' => 2,
+        'last_bounce_at' => nil,
+        'last_bounce_summary' => nil,
+        'last_suppression_bounce_at' => nil,
+        'last_transient_bounce_at' => nil,
+        'last_transient_bounce_summary' => nil
       })
     end
 

spec/models/communication_channel_spec.rb

@@ -230,6 +230,56 @@ describe CommunicationChannel do
     expect(@cc.reload).to be_active
   end
 
+  describe '#last_bounce_summary' do
+    it 'gets the diagnostic code' do
+      user = User.create!
+      cc = user.communication_channels.create!(path: 'path@example.com') do |cc|
+        cc.last_bounce_details = {
+          'bouncedRecipients' => [
+            {
+              'diagnosticCode' => 'stuff and things'
+            }
+          ]
+        }
+      end
+
+      expect(cc.last_bounce_summary).to eq('stuff and things')
+    end
+
+    it "doesn't fail when there isn't a last bounce" do
+      user = User.create!
+      cc = user.communication_channels.create!(path: 'path@example.com')
+
+      expect(cc.last_bounce_details).to be_nil
+      expect(cc.last_bounce_summary).to be_nil
+    end
+  end
+
+  describe '#last_transient_bounce_summary' do
+    it 'gets the diagnostic code' do
+      user = User.create!
+      cc = user.communication_channels.create!(path: 'path@example.com') do |cc|
+        cc.last_transient_bounce_details = {
+          'bouncedRecipients' => [
+            {
+              'diagnosticCode' => 'stuff and things'
+            }
+          ]
+        }
+      end
+
+      expect(cc.last_transient_bounce_summary).to eq('stuff and things')
+    end
+
+    it "doesn't fail when there isn't a last transient bounce" do
+      user = User.create!
+      cc = user.communication_channels.create!(path: 'path@example.com')
+
+      expect(cc.last_transient_bounce_details).to be_nil
+      expect(cc.last_transient_bounce_summary).to be_nil
+    end
+  end
+
   describe "merge candidates" do
     let_once(:user1) { User.create! }
     let_once(:cc1) { user1.communication_channels.create!(:path => 'jt@instructure.com') }