Ignore has_annotations? when new_annotations is enabled

We no longer care if the doc was created with annotations required.
The new DocViewer can handle annotating :allthethings:

Fixes RD-4229

Test plan:

Submit a box doc for an assignment
enable new_annotations and supports annotations
load doc in speed grader, you should be able to annotate it

Change-Id: I5dd594d338c7b1e60e8b15ec3021dd19bb4287c1
Reviewed-on: https://gerrit.instructure.com/116846
Tested-by: Jenkins
QA-Review: Caleb Guanzon <cguanzon@instructure.com>
Reviewed-by: Shahbaz Javeed <sjaveed@instructure.com>
Reviewed-by: Josh Orr <jgorr@instructure.com>
Product-Review: Brad Horrocks <bhorrocks@instructure.com>

app/controllers/canvadoc_sessions_controller.rb

@@ -38,6 +38,7 @@ class CanvadocSessionsController < ApplicationController
         opts[:preferred_plugins].unshift Canvadocs::RENDER_O365
       end
 
+      opts[:enable_annotations] = blob["enable_annotations"]
       attachment.submit_to_canvadocs(1, opts) unless attachment.canvadoc_available?
       url = attachment.canvadoc.session_url(opts.merge(user: @current_user))
 

app/controllers/crocodoc_sessions_controller.rb

@@ -34,6 +34,7 @@ class CrocodocSessionsController < ApplicationController
       crocodoc = attachment.crocodoc_document
       url = crocodoc.session_url(:user => @current_user,
                                  :annotations => annotations,
+                                 :enable_annotations => blob["enable_annotations"],
                                  :crocodoc_ids => blob["crocodoc_ids"])
 
       # For the purposes of reporting student viewership, we only

app/helpers/attachment_helper.rb

@@ -19,6 +19,7 @@
 module AttachmentHelper
   # returns a string of html attributes suitable for use with $.loadDocPreview
   def doc_preview_attributes(attachment, attrs={})
+    enable_annotations = attrs.delete(:enable_annotations)
     if attachment.crocodoc_available?
       begin
         attrs[:crocodoc_session_url] = attachment.crocodoc_url(@current_user, attrs[:crocodoc_ids])
@@ -26,7 +27,7 @@ module AttachmentHelper
         Canvas::Errors.capture_exception(:crocodoc, e)
       end
     elsif attachment.canvadocable?
-      attrs[:canvadoc_session_url] = attachment.canvadoc_url(@current_user)
+      attrs[:canvadoc_session_url] = attachment.canvadoc_url(@current_user, enable_annotations: enable_annotations)
     end
     attrs[:attachment_id] = attachment.id
     attrs[:mimetype] = attachment.mimetype

app/models/assignment/speed_grader.rb

@@ -203,7 +203,7 @@ class Assignment
                   end
                   a.as_json(only: attachment_fields,
                             methods: [:view_inline_ping_url]).tap do |json|
-                    json[:attachment][:canvadoc_url] = a.canvadoc_url(@user)
+                    json[:attachment][:canvadoc_url] = a.canvadoc_url(@user, enable_annotations: true)
                     json[:attachment][:crocodoc_url] = a.crocodoc_url(@user, crocodoc_user_ids)
                     json[:attachment][:submitted_to_crocodoc] = a.crocodoc_document.present?
                     json[:attachment][:hijack_crocodoc_session] = a.crocodoc_document&.should_migrate_to_canvadocs?

app/models/attachment.rb

@@ -1640,27 +1640,31 @@ class Attachment < ActiveRecord::Base
     "/#{context_url_prefix}/files/#{self.id}/inline_view"
   end
 
-  def canvadoc_url(user)
+  def canvadoc_url(user, opts={})
     return unless canvadocable?
-    "/api/v1/canvadoc_session?#{preview_params(user, "canvadoc")}"
+    "/api/v1/canvadoc_session?#{preview_params(user, "canvadoc", opts)}"
   end
 
   def crocodoc_url(user, crocodoc_ids = nil)
+    opts = {
+      enable_annotations: true
+    }
+    opts[:crocodoc_ids] = crocodoc_ids if crocodoc_ids.present?
     return unless crocodoc_available?
-    "/api/v1/crocodoc_session?#{preview_params(user, "crocodoc", crocodoc_ids)}"
+    "/api/v1/crocodoc_session?#{preview_params(user, "crocodoc", opts)}"
   end
 
   def previewable_media?
     self.content_type && self.content_type.match(/\A(video|audio)/)
   end
 
-  def preview_params(user, type, crocodoc_ids = nil)
-    h = {
+  def preview_params(user, type, opts = {})
+    h = opts.merge({
       user_id: user.try(:global_id),
       attachment_id: id,
       type: type
-    }
-    h.merge!(crocodoc_ids: crocodoc_ids) if crocodoc_ids.present?
+    })
+
     blob = h.to_json
     hmac = Canvas::Security.hmac_sha1(blob)
     "blob=#{URI.encode blob}&hmac=#{URI.encode hmac}"

app/models/canvadoc.rb

@@ -57,6 +57,13 @@ class Canvadoc < ActiveRecord::Base
     !!(document_id && process_state != 'error' && Canvadocs.enabled?)
   end
 
+  def has_annotations?
+    account_context = attachment.context.try(:account)
+    account_context ||= attachment.context.try(:root_account)
+    new_annotations_enabled = account_context&.feature_enabled?(:new_annotations)
+    new_annotations_enabled || annotations == true
+  end
+
   def self.mime_types
     JSON.parse Setting.get('canvadoc_mime_types', %w[
       application/excel

app/views/submissions/show_preview.html.erb

@@ -159,7 +159,7 @@
             %>
           <% end %>
           <% if attachment.crocodoc_available? || attachment.canvadocable? %>
-            <a href=<%= preview_url %> data-tooltip title="<%= preview_document %>" class="modal_preview_link Button--link" role="button" data-attachment_id="<%= attachment.id %>" data-submission_id="<%= @submission.id %>" data-dialog-title="<%= attachment.display_name %>" <%= doc_preview_attributes(attachment, crocodoc_ids: @crocodoc_ids) %>><%= button_text %></a>
+            <a href=<%= preview_url %> data-tooltip title="<%= preview_document %>" class="modal_preview_link Button--link" role="button" data-attachment_id="<%= attachment.id %>" data-submission_id="<%= @submission.id %>" data-dialog-title="<%= attachment.display_name %>" <%= doc_preview_attributes(attachment, enable_annotations: true, crocodoc_ids: @crocodoc_ids) %>><%= button_text %></a>
           <% end %>
         </div>
 

lib/canvadocs/session.rb

@@ -5,7 +5,8 @@ module Canvadocs
 
     def canvadocs_session_url(opts = {})
       user = opts.delete(:user)
-      opts.merge! canvadoc_permissions_for_user(user)
+      enable_annotations = opts.delete(:enable_annotations)
+      opts.merge! canvadoc_permissions_for_user(user, enable_annotations)
       opts[:url] = attachment.authenticated_s3_url(expires_in: 7.days)
       opts[:locale] = I18n.locale || I18n.default_locale
 
@@ -20,8 +21,8 @@ module Canvadocs
     end
     private :canvadocs_api
 
-    def canvadoc_permissions_for_user(user)
-      return {} unless canvadocs_can_annotate? user
+    def canvadoc_permissions_for_user(user, enable_annotations)
+      return {} unless enable_annotations && canvadocs_can_annotate?(user)
 
       annotation_context = "default"
       if ApplicationController.respond_to?(:test_cluster?) && ApplicationController.test_cluster?
@@ -57,9 +58,8 @@ module Canvadocs
     private :canvadoc_permissions_for_user
 
     def canvadocs_can_annotate?(user)
-      user && has_annotations?
+      user.present?
     end
     private :canvadocs_can_annotate?
-
   end
 end

spec/models/attachment_spec.rb

@@ -1721,15 +1721,6 @@ describe Attachment do
     end
   end
 
-  describe "preview_params" do
-    it "includes crocodoc_ids only when a whitelist is given" do
-      course_factory :active_all => true
-      att = attachment_model
-      expect(att.send :preview_params, @teacher, 'document/msword').not_to include 'crocodoc_ids'
-      expect(att.send :preview_params, @teacher, 'document/msword', [1]).to include 'crocodoc_ids'
-    end
-  end
-
   describe '#ajax_upload_params' do
     it 'returns the attachment filename in the upload params' do
       attachment_model filename: 'test.txt'

spec/models/canvadoc_spec.rb

@@ -78,7 +78,7 @@ describe 'Canvadoc' do
       @doc.has_annotations = true
       canvadocs_api = @doc.send(:canvadocs_api)
       expect(canvadocs_api).to receive(:session).with(anything, hash_including(annotation_context: 'default')).and_call_original
-      @doc.session_url(user: @attachment.user)
+      @doc.session_url(user: @attachment.user, enable_annotations: true)
     end
 
     it "Creates test context for annotation session" do
@@ -91,7 +91,7 @@ describe 'Canvadoc' do
       canvadocs_api = @doc.send(:canvadocs_api)
 
       expect(canvadocs_api).to receive(:session).with(anything, hash_including(annotation_context: 'default-super-secret-testing')).and_call_original
-      @doc.session_url(user: @attachment.user)
+      @doc.session_url(user: @attachment.user, enable_annotations: true)
     end
 
     it "Session creation sends users crocodoc id" do
@@ -101,7 +101,7 @@ describe 'Canvadoc' do
       canvadocs_api = @doc.send(:canvadocs_api)
 
       expect(canvadocs_api).to receive(:session).with(anything, hash_including(user_crocodoc_id: @attachment.user.crocodoc_id)).and_call_original
-      @doc.session_url(user: @attachment.user)
+      @doc.session_url(user: @attachment.user, enable_annotations: true)
     end
   end