api doc request parameter formatting

refs CNVS-26515

Request params are now formatted in an HTML table, with separate fields
for name, type, required/optional, valid enum values, and description.
This is at the same time more compact and more readable.

To parse this information out of what is actually free-form text
comments, I'm re-using the OpenAPI (swagger) parsing code, which I
tweaked and simplified slightly. Eventually I'd like to see the whole
API documentation generated from the OpenAPI spec, and then we can
slowly move away from using free-form code comments to more declarative
code constructs, but still generate an OpenAPI spec and get the same API
docs.

test plan: Generate the api documentation, and view request parameter
information. you should see the new formatting.

Change-Id: Ida00869d96a9b2d9fa84c29079ddf2511a2d5917
Reviewed-on: https://gerrit.instructure.com/70485
Reviewed-by: Simon Williams <simon@instructure.com>
Tested-by: Jenkins
Product-Review: Brian Palmer <brianp@instructure.com>
QA-Review: Brian Palmer <brianp@instructure.com>

app/controllers/assignment_overrides_controller.rb

@@ -171,19 +171,19 @@ class AssignmentOverridesController < ApplicationController
   #   section of the assignment's course not already targetted by a different
   #   override.
   #
-  # @argument assignment_override[due_at] [Timestamp] The day/time
+  # @argument assignment_override[due_at] [DateTime] The day/time
   #   the overridden assignment is due. Accepts times in ISO 8601 format, e.g.
   #   2014-10-21T18:48:00Z. If absent, this override will not affect due date.
   #   May be present but null to indicate the override removes any previous due
   #   date.
   #
-  # @argument assignment_override[unlock_at] [Timestamp] The day/time
+  # @argument assignment_override[unlock_at] [DateTime] The day/time
   #   the overridden assignment becomes unlocked. Accepts times in ISO 8601
   #   format, e.g. 2014-10-21T18:48:00Z. If absent, this override will not
   #   affect the unlock date. May be present but null to indicate the override
   #   removes any previous unlock date.
   #
-  # @argument assignment_override[lock_at] [Timestamp] The day/time
+  # @argument assignment_override[lock_at] [DateTime] The day/time
   #   the overridden assignment becomes locked. Accepts times in ISO 8601
   #   format, e.g. 2014-10-21T18:48:00Z. If absent, this override will not
   #   affect the lock date. May be present but null to indicate the override
@@ -230,19 +230,19 @@ class AssignmentOverridesController < ApplicationController
   # @argument assignment_override[title] [String] The title of an adhoc
   #   assignment override. Ignored unless the override being updated is adhoc.
   #
-  # @argument assignment_override[due_at] [Timestamp] The day/time
+  # @argument assignment_override[due_at] [DateTime] The day/time
   #   the overridden assignment is due. Accepts times in ISO 8601 format, e.g.
   #   2014-10-21T18:48:00Z. If absent, this override will not affect due date.
   #   May be present but null to indicate the override removes any previous due
   #   date.
   #
-  # @argument assignment_override[unlock_at] [Timestamp] The day/time
+  # @argument assignment_override[unlock_at] [DateTime] The day/time
   #   the overridden assignment becomes unlocked. Accepts times in ISO 8601
   #   format, e.g. 2014-10-21T18:48:00Z. If absent, this override will not
   #   affect the unlock date. May be present but null to indicate the override
   #   removes any previous unlock date.
   #
-  # @argument assignment_override[lock_at] [Timestamp] The day/time
+  # @argument assignment_override[lock_at] [DateTime] The day/time
   #   the overridden assignment becomes locked. Accepts times in ISO 8601
   #   format, e.g. 2014-10-21T18:48:00Z. If absent, this override will not
   #   affect the lock date. May be present but null to indicate the override

app/controllers/assignments_api_controller.rb

@@ -540,9 +540,8 @@ class AssignmentsApiController < ApplicationController
   #   Apply assignment overrides for each assignment, defaults to true.
   # @argument needs_grading_count_by_section [Boolean]
   #   Split up "needs_grading_count" by sections into the "needs_grading_count_by_section" key, defaults to false
-  # @argument bucket [String]
+  # @argument bucket [String, "past"|"overdue"|"undated"|"ungraded"|"upcoming"|"future"]
   #   If included, only return certain assignments depending on due date and submission status.
-  #   Valid buckets are "past", "overdue", "undated", "ungraded", "upcoming", and "future".
   # @returns [Assignment]
   def index
     error_or_array= get_assignments(@current_user)
@@ -786,15 +785,15 @@ class AssignmentsApiController < ApplicationController
   #  The strategy used for grading the assignment.
   #  The assignment is ungraded if this field is omitted.
   #
-  # @argument assignment[due_at] [Timestamp]
+  # @argument assignment[due_at] [DateTime]
   #   The day/time the assignment is due.
   #   Accepts times in ISO 8601 format, e.g. 2014-10-21T18:48:00Z.
   #
-  # @argument assignment[lock_at] [Timestamp]
+  # @argument assignment[lock_at] [DateTime]
   #   The day/time the assignment is locked after.
   #   Accepts times in ISO 8601 format, e.g. 2014-10-21T18:48:00Z.
   #
-  # @argument assignment[unlock_at] [Timestamp]
+  # @argument assignment[unlock_at] [DateTime]
   #   The day/time the assignment is unlocked.
   #   Accepts times in ISO 8601 format, e.g. 2014-10-21T18:48:00Z.
   #
@@ -930,15 +929,15 @@ class AssignmentsApiController < ApplicationController
   #  The strategy used for grading the assignment.
   #  The assignment is ungraded if this field is omitted.
   #
-  # @argument assignment[due_at] [Timestamp]
+  # @argument assignment[due_at] [DateTime]
   #   The day/time the assignment is due.
   #   Accepts times in ISO 8601 format, e.g. 2014-10-21T18:48:00Z.
   #
-  # @argument assignment[lock_at] [Timestamp]
+  # @argument assignment[lock_at] [DateTime]
   #   The day/time the assignment is locked after.
   #   Accepts times in ISO 8601 format, e.g. 2014-10-21T18:48:00Z.
   #
-  # @argument assignment[unlock_at] [Timestamp]
+  # @argument assignment[unlock_at] [DateTime]
   #   The day/time the assignment is unlocked.
   #   Accepts times in ISO 8601 format, e.g. 2014-10-21T18:48:00Z.
   #

app/controllers/authentication_audit_api_controller.rb

@@ -78,7 +78,7 @@ class AuthenticationAuditApiController < AuditorApiController
   # @argument start_time [DateTime]
   #   The beginning of the time range from which you want events.
   #
-  # @argument end_time [Datetime]
+  # @argument end_time [DateTime]
   #   The end of the time range from which you want events.
   #
   def for_login
@@ -95,10 +95,10 @@ class AuthenticationAuditApiController < AuditorApiController
   #
   # List authentication events for a given account.
   #
-  # @argument start_time [Datetime]
+  # @argument start_time [DateTime]
   #   The beginning of the time range from which you want events.
   #
-  # @argument end_time [Datetime]
+  # @argument end_time [DateTime]
   #   The end of the time range from which you want events.
   #
   def for_account
@@ -115,10 +115,10 @@ class AuthenticationAuditApiController < AuditorApiController
   #
   # List authentication events for a given user.
   #
-  # @argument start_time [Datetime]
+  # @argument start_time [DateTime]
   #   The beginning of the time range from which you want events.
   #
-  # @argument end_time [Datetime]
+  # @argument end_time [DateTime]
   #   The end of the time range from which you want events.
   #
   def for_user

app/controllers/course_audit_api_controller.rb

@@ -176,7 +176,7 @@ class CourseAuditApiController < AuditorApiController
   # @argument start_time [DateTime]
   #   The beginning of the time range from which you want events.
   #
-  # @argument end_time [Datetime]
+  # @argument end_time [DateTime]
   #   The end of the time range from which you want events.
   #
   # @returns [CourseEvent]

app/controllers/grade_change_audit_api_controller.rb

@@ -123,7 +123,7 @@ class GradeChangeAuditApiController < AuditorApiController
   # @argument start_time [DateTime]
   #   The beginning of the time range from which you want events.
   #
-  # @argument end_time [Datetime]
+  # @argument end_time [DateTime]
   #   The end of the time range from which you want events.
   #
   # @returns [GradeChangeEvent]
@@ -148,7 +148,7 @@ class GradeChangeAuditApiController < AuditorApiController
   # @argument start_time [DateTime]
   #   The beginning of the time range from which you want events.
   #
-  # @argument end_time [Datetime]
+  # @argument end_time [DateTime]
   #   The end of the time range from which you want events.
   #
   # @returns [GradeChangeEvent]
@@ -170,7 +170,7 @@ class GradeChangeAuditApiController < AuditorApiController
   # @argument start_time [DateTime]
   #   The beginning of the time range from which you want events.
   #
-  # @argument end_time [Datetime]
+  # @argument end_time [DateTime]
   #   The end of the time range from which you want events.
   #
   # @returns [GradeChangeEvent]
@@ -195,7 +195,7 @@ class GradeChangeAuditApiController < AuditorApiController
   # @argument start_time [DateTime]
   #   The beginning of the time range from which you want events.
   #
-  # @argument end_time [Datetime]
+  # @argument end_time [DateTime]
   #   The end of the time range from which you want events.
   #
   # @returns [GradeChangeEvent]

app/controllers/quizzes/quizzes_api_controller.rb

@@ -384,13 +384,13 @@ class Quizzes::QuizzesApiController < ApplicationController
   #   until they submit the last attempt for the quiz.
   #   Defaults to false.
   #
-  # @argument quiz[show_correct_answers_at] [Timestamp]
+  # @argument quiz[show_correct_answers_at] [DateTime]
   #   Only valid if show_correct_answers=true
   #   If set, the correct answers will be visible by students only after this
   #   date, otherwise the correct answers are visible once the student hands in
   #   their quiz submission.
   #
-  # @argument quiz[hide_correct_answers_at] [Timestamp]
+  # @argument quiz[hide_correct_answers_at] [DateTime]
   #   Only valid if show_correct_answers=true
   #   If set, the correct answers will stop being visible once this date has
   #   passed. Otherwise, the correct answers will be visible indefinitely.
@@ -431,15 +431,15 @@ class Quizzes::QuizzesApiController < ApplicationController
   #   For no IP filter restriction, set to null.
   #   Defaults to null.
   #
-  # @argument quiz[due_at] [Timestamp]
+  # @argument quiz[due_at] [DateTime]
   #   The day/time the quiz is due.
   #   Accepts times in ISO 8601 format, e.g. 2011-10-21T18:48Z.
   #
-  # @argument quiz[lock_at] [Timestamp]
+  # @argument quiz[lock_at] [DateTime]
   #   The day/time the quiz is locked for students.
   #   Accepts times in ISO 8601 format, e.g. 2011-10-21T18:48Z.
   #
-  # @argument quiz[unlock_at] [Timestamp]
+  # @argument quiz[unlock_at] [DateTime]
   #   The day/time the quiz is unlocked for students.
   #   Accepts times in ISO 8601 format, e.g. 2011-10-21T18:48Z.
   #

app/controllers/terms_api_controller.rb

@@ -70,7 +70,7 @@ class TermsApiController < ApplicationController
   #
   # Return all of the terms in the account.
   #
-  # @argument workflow_state[] [String, 'active'| 'deleted'| 'all']
+  # @argument workflow_state[] [String, "active"|"deleted"|"all"]
   #   If set, only returns terms that are in the given state.
   #   Defaults to 'active'.
   #

app/controllers/terms_controller.rb

@@ -38,11 +38,11 @@ class TermsController < ApplicationController
   # @argument enrollment_term[name] [String]
   #   The name of the term.
   #
-  # @argument enrollment_term[start_at] [Timestamp]
+  # @argument enrollment_term[start_at] [DateTime]
   #   The day/time the term starts.
   #   Accepts times in ISO 8601 format, e.g. 2015-01-10T18:48:00Z.
   #
-  # @argument enrollment_term[end_at] [Timestamp]
+  # @argument enrollment_term[end_at] [DateTime]
   #   The day/time the term ends.
   #   Accepts times in ISO 8601 format, e.g. 2015-01-10T18:48:00Z.
   #
@@ -63,11 +63,11 @@ class TermsController < ApplicationController
   # @argument enrollment_term[name] [String]
   #   The name of the term.
   #
-  # @argument enrollment_term[start_at] [Timestamp]
+  # @argument enrollment_term[start_at] [DateTime]
   #   The day/time the term starts.
   #   Accepts times in ISO 8601 format, e.g. 2015-01-10T18:48:00Z.
   #
-  # @argument enrollment_term[end_at] [Timestamp]
+  # @argument enrollment_term[end_at] [DateTime]
   #   The day/time the term ends.
   #   Accepts times in ISO 8601 format, e.g. 2015-01-10T18:48:00Z.
   #

doc/api/fulldoc/html/css/common.css

@@ -1,3 +1,7 @@
+p {
+  line-height: 1.5;
+}
+
 .label {
   display: inline-block;
   padding: 1px 4px;
@@ -80,7 +84,7 @@ body {
 }
 #content {
   margin: 10px 20px 0 220px;
-  max-width: 800px;
+  max-width: 1280px;
 }
 
 #header {
@@ -254,6 +258,9 @@ pre.code {
   background-color: #2A404D;
   color: #fff;
 }
+pre.code code {
+  overflow: scroll;
+}
 
 div.syntaxhighlighter {
   padding: 15px;
@@ -294,3 +301,35 @@ div.warning-message {
   border-bottom-color: rgba(0,0,0,0.5);
   border-radius: 3px;
 }
+
+code.enum {
+  color: #d14;
+  padding: 0 4px;
+  margin-bottom: 2px;
+  background-color: #f7f7f9;
+  border: 1px solid #e1e1e8;
+}
+
+table.request-params {
+  table-layout: fixed;
+  width: 100%;
+}
+table.request-params, table.request-params th, table.request-params td {
+  border: 0;
+  text-align: left;
+}
+table.request-params td {
+  border-bottom: 1px solid #ccc;
+  vertical-align: text-top;
+}
+.param-name {
+  width: 20em;
+}
+.param-req {
+  width: 5em;
+}
+.param-type {
+  width: 10em;
+}
+.param-desc {
+}

doc/api/fulldoc/html/swagger/argument_view.rb

@@ -15,50 +15,26 @@ class ArgumentView < HashView
   end
 
   def parse_line(line)
-    clean_line = line.gsub(/\s+/m, " ")
-    name, remaining = clean_line.scan(/^([^\s]+)(.*)$/).first
-    name.strip! if @name
-    if remaining
-      type, desc = split_type_desc(remaining)
-      # type = format(type.strip.gsub('[', '').gsub(']', '')) if type
-      type.strip! if type
-      desc.strip! if desc
-    end
-    [clean_line, name, type, desc]
+    name, remaining = (line || "").split(/\s/, 2)
+    raise(ArgumentError, "param name missing:\n#{line}") unless name
+    name.strip!
+    type, desc = split_type_desc(remaining || "")
+    type.strip!
+    desc.strip!
+    [line, name, type, desc]
   end
 
-  # Atrocious use of regex to parse out type signatures such as:
-  # "[[Integer], Optional] The IDs of the override's target students."
   def split_type_desc(str)
-    type_desc_parts_to_pair(
-      str.strip.
-      # turn "] ," into "],"
-      gsub(/\]\s+,/, '],').
-      # put "|||" between type and desc
-      sub(/[^,] /){ |s| s[0] == ']' ? s[0] + '|||' : s }.
-      # split on "|||"
-      split('|||')
-    )
+    # This regex is impossible to read, basically we're splitting the string up
+    # into the first [bracketed] section, which might contain internal brackets,
+    # and then the rest of the string.
+    md = str.strip.match(%r{\A(\[[\w ,\[\]\|"]+\])?\s*(.+)?}m)
+    [md[1] || DEFAULT_TYPE, md[2] || DEFAULT_DESC]
   end
 
-  def type_desc_parts_to_pair(parts)
-    case parts.size
-    when 0 then [DEFAULT_TYPE, DEFAULT_DESC]
-    when 1 then
-      if parts.first.include?('[') and parts.first.include?(']')
-        [parts.first, DEFAULT_DESC]
-      else
-        [DEFAULT_TYPE, parts.first]
-      end
-    when 2 then
-      parts
-    else
-      raise "Too many parts while splitting type and description: #{parts.inspect}"
-    end
-  end
-
-  def name
-    format(@name.gsub('[]', ''))
+  def name(json: true)
+    name = json ? @name.gsub('[]', '') : @name
+    format(name)
   end
 
   def desc
@@ -102,7 +78,8 @@ class ArgumentView < HashView
   end
 
   def swagger_type
-    (types.first || 'string').downcase
+    type = (types.first || 'string')
+    builtin?(type) ? type.downcase : type
   end
 
   def swagger_format
@@ -132,7 +109,7 @@ class ArgumentView < HashView
   end
 
   def builtin?(type)
-    ["string", "integer"].include?(type)
+    ["string", "integer", "boolean", "number"].include?(type.downcase)
   end
 
   def to_swagger
@@ -165,5 +142,5 @@ class ArgumentView < HashView
       "types"    => types,
       "optional" => optional?,
     }
-  end    
-end
+  end
+end

doc/api/layout/html/headers.erb

@@ -12,6 +12,7 @@ $(function() {
   $('pre.code.html').addClass('prettyprint language-html');
   $('pre.code.xml').addClass('prettyprint language-xml');
   $('pre.code.json').addClass('prettyprint language-js');
+  $('pre.code.javascript').addClass('prettyprint language-js');
   prettyPrint();
 });
 </script>

doc/api/tags/html/request_parameters.erb

@@ -0,0 +1,32 @@
+<h4>Request Parameters:</h4>
+<table class="request-params">
+  <thead>
+    <tr>
+      <th class="param-name">Parameter</th>
+      <th class="param-req"></th>
+      <th class="param-type">Type</th>
+      <th class="param-desc">Description</th>
+    </tr>
+  </thead>
+  <tbody>
+    <% @request_parameters.each do |param| %>
+    <tr class="request-param">
+      <td><%= h param.name(json: false) %></td>
+      <td>
+        <% if param.required? %>
+        Required
+        <% end %>
+      </td>
+      <td><%= h param.swagger_type %></td>
+      <td>
+        <%= htmlify param.desc %>
+        <% if param.enums.present? %>
+        <p>
+          Allowed values: <%= param.enums.map { |v| "<code class=enum>#{v}</code>" }.join(", ") %>
+        </p>
+        <% end %>
+      </td>
+    </tr>
+    <% end %>
+  </tbody>
+</table>

doc/api/tags/setup.rb

@@ -35,7 +35,9 @@ def response_field
 end
 
 def argument
-  generic_tag :argument, :no_types => false, :label => "Request Parameters"
+  return unless object.has_tag?(:argument)
+  @request_parameters = object.tags(:argument).map { |t| ArgumentView.new(t.text) }
+  erb('request_parameters')
 end
 
 def returns
@@ -64,4 +66,3 @@ def generic_tag(name, opts = {})
   @no_names, @no_types = nil, nil
   out
 end
-

spec/apis/swagger/argument_view_spec.rb

@@ -3,7 +3,7 @@ require 'argument_view'
 
 describe ArgumentView do
   context "type splitter" do
-    let(:view) { ArgumentView.new "" }
+    let(:view) { ArgumentView.new "arg [String]" }
 
     it "accepts no type" do
       expect(view.split_type_desc("")).to eq(
@@ -31,11 +31,10 @@ describe ArgumentView do
   end
 
   context "line parser" do
-    let(:view) { ArgumentView.new "" }
+    let(:view) { ArgumentView.new "arg [String]" }
 
-    it "compacts whitespace" do
-      parsed = view.parse_line("arg  \t[String] desc")
-      expect(parsed).to eq ["arg [String] desc", "arg", "[String]", "desc"]
+    it "raises on missing param name" do
+      expect { view.parse_line("") }.to raise_error(ArgumentError)
     end
 
     it "parses without desc" do
@@ -80,4 +79,4 @@ describe ArgumentView do
       expect(view.required?).to be_truthy
     end
   end
-end
+end