fix: serialize range fields as bracket literals, symmetric with writes

app/graphql/types/concerns/range_literal_fields.rb

@@ -0,0 +1,29 @@
+# frozen_string_literal: true
+
+module Types
+  module Concerns
+    # Resolver methods for the 7 numeric-range fields shared by PlantType and
+    # VarietyType. The `field :x_range, String, ...` declarations already
+    # exist on each including type; this concern only supplies the resolver
+    # methods, so every range field renders through RangeLiteral.serialize --
+    # the same canonical bracket-literal format
+    # (Mutations::Concerns::RangeLiteralValidation) that create/update
+    # mutations accept -- instead of graphql-ruby's default String
+    # coercion, which falls back to Ruby's Range#to_s ("5.5..7.0",
+    # "0...Infinity").
+    #
+    # Sources the field list from RangeLiteralValidation::RANGE_FIELDS so
+    # there is exactly one list of "the 7 range fields" in the codebase.
+    module RangeLiteralFields
+      extend ActiveSupport::Concern
+
+      included do
+        Mutations::Concerns::RangeLiteralValidation::RANGE_FIELDS.each do |field|
+          define_method(field) do
+            RangeLiteral.serialize(object.public_send(field))
+          end
+        end
+      end
+    end
+  end
+end

app/graphql/types/plant_type.rb

@@ -8,6 +8,7 @@ class PlantType < Types::BaseObject # rubocop:disable Metrics/ClassLength
 
     include Types::Concerns::CapabilityFields
     include Types::Concerns::DraftFields
+    include Types::Concerns::RangeLiteralFields
 
     description 'A plant is a crop species available through the Plant API.'
 

app/graphql/types/record_draft_info_type.rb

@@ -5,9 +5,11 @@ module Types
   class RecordDraftInfoType < Types::BaseObject
     field :updated_at, GraphQL::Types::ISO8601DateTime, null: false
     field :author, String, null: true,
-                           description: 'Display name of the principal who started this draft.'
+                           description: 'Display name of the principal who started this draft, falling back to ' \
+                                        'their email when no display name is on file.'
     field :last_editor, String, null: true,
-                                description: 'Display name of the principal who last edited it.'
+                                description: 'Display name of the principal who last edited it, falling back to ' \
+                                             'their email when no display name is on file.'
     field :changed_fields, [String], null: false,
                                      description: 'Attribute names this draft changes.'
     field :is_stale, Boolean, null: false,
@@ -16,12 +18,18 @@ class RecordDraftInfoType < Types::BaseObject
                                            'every resolution -- avoid requesting it across large lists; batching it ' \
                                            'is a tracked follow-up.'
 
+    # display_name is not always populated: resolve_actor (application_controller.rb)
+    # never passes it when it upserts a Principal from a JWT claim, so it is nil for
+    # most real users today and this field would otherwise render "Unknown" client
+    # side. Fall back to email, which this type's callers (editors, gated by
+    # DraftFields#draft requiring update?) already see elsewhere on the record via
+    # ownedBy/createdBy, so this is no new exposure.
     def author
-      object.author&.display_name
+      object.author&.display_name.presence || object.author&.email
     end
 
     def last_editor
-      object.last_editor&.display_name
+      object.last_editor&.display_name.presence || object.last_editor&.email
     end
 
     def is_stale

app/graphql/types/variety_type.rb

@@ -8,6 +8,7 @@ class VarietyType < Types::BaseObject # rubocop:disable Metrics/ClassLength
 
     include Types::Concerns::CapabilityFields
     include Types::Concerns::DraftFields
+    include Types::Concerns::RangeLiteralFields
 
     description 'A variety represents a more precisely defined subgroup of plants with a common set of characteristics.'
 

app/services/range_literal.rb

@@ -0,0 +1,90 @@
+# frozen_string_literal: true
+
+# Serializes an ActiveRecord Range attribute (backed by a Postgres int4range
+# or numrange column) back into the bracket-literal string format the write
+# side (Mutations::Concerns::RangeLiteralValidation::RANGE_LITERAL) accepts,
+# e.g. "[10,20]", "[10,]", "[,100]". Symmetric with writes: what an editor
+# typed on create/update is what they see again on read.
+#
+# Before this existed, the 7 range fields on PlantType/VarietyType rendered
+# through graphql-ruby's default String coercion, i.e. Ruby's Range#to_s --
+# "5.5..7.0", "600...1201", "0...Infinity" in production. Writes accepted
+# bracket literals; reads emitted Ruby Range syntax. The admin SPA's range
+# parser (and the documented write format) only understands bracket
+# literals, so every existing value rendered as a blank input.
+#
+# This module is stateless -- all methods are module functions.
+module RangeLiteral
+  module_function
+
+  # range - a Ruby Range as ActiveRecord's PostgreSQL::OID::Range casts it
+  # (nil endpoints from an unbounded side are represented as
+  # Float::INFINITY / -Float::INFINITY, never literal nil, but nil is
+  # tolerated defensively). Returns nil for a nil range, otherwise a bracket
+  # literal string.
+  def serialize(range)
+    return nil if range.nil?
+
+    lower = range.begin
+    upper = range.end
+    exclude_end = range.exclude_end?
+    upper_unbounded = unbounded?(upper)
+
+    # Postgres canonicalizes discrete ranges (int4range) to an inclusive
+    # lower / exclusive upper bound -- an editor who wrote the inclusive
+    # literal "[500,2000]" gets back the Ruby Range 500...2001 once the row
+    # round-trips through the database. Numeric ranges (numrange) are
+    # continuous, so Postgres stores exactly the inclusivity it was given
+    # and this branch never fires for them.
+    if integer_range?(lower, upper) && !upper_unbounded && exclude_end
+      upper -= 1
+      exclude_end = false
+    end
+
+    # An unbounded upper bound always renders as an empty side with a
+    # closing "]" ("[10,]"), matching the literal the write side expects for
+    # "no upper limit" -- never "[10,)", which would read as a real
+    # (if oddly punctuated) exclusive bound rather than "unset".
+    upper_bracket = upper_unbounded || !exclude_end ? ']' : ')'
+
+    "[#{format_bound(lower)},#{format_bound(upper)}#{upper_bracket}"
+  end
+
+  # A Range's lower bound is always inclusive here: Ruby's Range class has
+  # no way to represent an open lower bound (assigning a Postgres literal
+  # with an open lower bound, e.g. "(5,10]", raises ArgumentError at cast
+  # time), so the opening bracket is always "[".
+  def unbounded?(bound)
+    bound.nil? || (bound.respond_to?(:infinite?) && bound.infinite?)
+  end
+  private_class_method :unbounded?
+
+  # int4range bounds cast to Integer; numrange bounds cast to BigDecimal.
+  # Either bound being an Integer is enough to identify a discrete range,
+  # since an unbounded side casts to Float::INFINITY regardless of subtype.
+  def integer_range?(lower, upper)
+    lower.is_a?(Integer) || upper.is_a?(Integer)
+  end
+  private_class_method :integer_range?
+
+  def format_bound(bound)
+    return '' if unbounded?(bound)
+    return bound.to_s if bound.is_a?(Integer)
+    return format_big_decimal(bound) if bound.is_a?(BigDecimal)
+
+    bound.to_s
+  end
+  private_class_method :format_bound
+
+  # BigDecimal#to_s defaults to scientific notation ("0.55e1" for 5.5), which
+  # is not a valid range-literal bound. #to_s('F') gives fixed-point
+  # notation instead. Formatting choice: a trailing ".0" is stripped, so a
+  # whole-number bound like 7.0 renders as "7" (matching what an editor
+  # would type for a whole number), while a fractional bound like 5.5 is
+  # left untouched. The round-trip spec asserts this exact choice.
+  def format_big_decimal(bound)
+    str = bound.to_s('F')
+    str.end_with?('.0') ? str.delete_suffix('.0') : str
+  end
+  private_class_method :format_big_decimal
+end

schema.graphql

@@ -7375,7 +7375,7 @@ type Query {
 
 type RecordDraftInfo {
   """
-  Display name of the principal who started this draft.
+  Display name of the principal who started this draft, falling back to their email when no display name is on file.
   """
   author: String
 
@@ -7392,7 +7392,7 @@ type RecordDraftInfo {
   isStale: Boolean!
 
   """
-  Display name of the principal who last edited it.
+  Display name of the principal who last edited it, falling back to their email when no display name is on file.
   """
   lastEditor: String
   updatedAt: ISO8601DateTime!

spec/mutations/range_literal_round_trip_spec.rb

@@ -0,0 +1,113 @@
+# frozen_string_literal: true
+
+require 'rails_helper'
+
+# Reads of the 7 range fields now go through RangeLiteral.serialize
+# (app/graphql/types/concerns/range_literal_fields.rb), so what a mutation
+# accepts is what a subsequent query returns -- previously reads fell back to
+# graphql-ruby's default String coercion, i.e. Ruby's Range#to_s
+# ("5.5..7.0", "0...Infinity"), which the admin SPA's range parser cannot
+# read back into its inputs.
+RSpec.describe 'range literal read/write symmetry', type: :graphql_mutation do
+  before :each do
+    Mobility.locale = nil
+  end
+
+  let(:current_user) { build(:user, :readwrite) }
+  let!(:plant) { create(:plant, owned_by: current_user.email, created_by: current_user.email) }
+  let(:plant_gid) { PlantApiSchema.id_from_object(plant, Plant, {}) }
+
+  def update(input)
+    query = <<~GRAPHQL
+      mutation($input: UpdatePlantInput!) {
+        updatePlant(input: $input) {
+          errors { field message code }
+          plant { uuid }
+        }
+      }
+    GRAPHQL
+    PlantApiSchema.execute(query, context: { current_user: current_user },
+                                  variables: { input: { plantId: plant_gid }.merge(input) })
+  end
+
+  def read(perspective: 'PUBLISHED')
+    query = <<~GRAPHQL
+      query($id: ID!, $perspective: Perspective) {
+        plant(id: $id, perspective: $perspective) {
+          nAccumulationRange
+          phRange
+        }
+      }
+    GRAPHQL
+    PlantApiSchema.execute(query, context: { current_user: current_user },
+                                  variables: { id: plant_gid, perspective: perspective })
+                  .dig('data', 'plant')
+  end
+
+  it 'round-trips an unbounded-upper integer literal' do
+    result = update(nAccumulationRange: '[10,]')
+    expect(result.dig('data', 'updatePlant', 'errors')).to eq([])
+
+    expect(read['nAccumulationRange']).to eq '[10,]'
+  end
+
+  it 'round-trips a finite inclusive integer literal, decrementing back from ' \
+     "Postgres's canonicalized exclusive-upper storage" do
+    result = update(nAccumulationRange: '[500,2000]')
+    expect(result.dig('data', 'updatePlant', 'errors')).to eq([])
+
+    # Confirms the canonicalization this spec exists to guard against: what
+    # is actually stored is the exclusive-upper form, not what was typed.
+    expect(plant.reload.n_accumulation_range).to eq(500...2001)
+    expect(read['nAccumulationRange']).to eq '[500,2000]'
+  end
+
+  it 'round-trips a numrange literal per the documented BigDecimal formatting choice ' \
+     '(trailing .0 stripped)' do
+    result = update(phRange: '[5.5,7.0]')
+    expect(result.dig('data', 'updatePlant', 'errors')).to eq([])
+
+    expect(read['phRange']).to eq '[5.5,7]'
+  end
+
+  describe 'DRAFT perspective' do
+    it 'returns the bracket literal for a staged (never-persisted-to-Postgres) range value' do
+      result = update(nAccumulationRange: '[500,2000]', saveAsDraft: true)
+      expect(result.dig('data', 'updatePlant', 'errors')).to eq([])
+
+      # The live row is untouched; only the draft carries the staged value.
+      expect(plant.reload.n_accumulation_range).not_to eq(500...2001)
+
+      expect(read(perspective: 'DRAFT')['nAccumulationRange']).to eq '[500,2000]'
+    end
+  end
+
+  describe 'VarietyType' do
+    let!(:variety) { create(:variety, owned_by: current_user.email, created_by: current_user.email) }
+    let(:variety_gid) { PlantApiSchema.id_from_object(variety, Variety, {}) }
+
+    it 'serializes range fields the same way as PlantType' do
+      mutation = <<~GRAPHQL
+        mutation($input: UpdateVarietyInput!) {
+          updateVariety(input: $input) {
+            errors { field message code }
+            variety { uuid }
+          }
+        }
+      GRAPHQL
+      result = PlantApiSchema.execute(mutation, context: { current_user: current_user },
+                                                variables: { input: { varietyId: variety_gid,
+                                                                      optimalAltitudeRange: '[500,2000]' } })
+      expect(result.dig('data', 'updateVariety', 'errors')).to eq([])
+
+      query = <<~GRAPHQL
+        query($id: ID!) {
+          variety(id: $id) { optimalAltitudeRange }
+        }
+      GRAPHQL
+      result = PlantApiSchema.execute(query, context: { current_user: current_user },
+                                             variables: { id: variety_gid })
+      expect(result.dig('data', 'variety', 'optimalAltitudeRange')).to eq '[500,2000]'
+    end
+  end
+end

spec/services/range_literal_spec.rb

@@ -0,0 +1,79 @@
+# frozen_string_literal: true
+
+require 'rails_helper'
+
+RSpec.describe RangeLiteral, type: :service do
+  describe '.serialize' do
+    it 'returns nil for a nil range' do
+      expect(described_class.serialize(nil)).to be_nil
+    end
+
+    context 'integer ranges (int4range)' do
+      it 'decrements a finite exclusive upper bound to the inclusive equivalent' do
+        # Postgres canonicalizes int4range to inclusive-lower/exclusive-upper,
+        # so an editor's "[500,2000]" round-trips through the database as the
+        # Ruby Range 500...2001.
+        range = Range.new(500, 2001, true)
+        expect(described_class.serialize(range)).to eq '[500,2000]'
+      end
+
+      it 'renders an unbounded upper as an empty side closed with ]' do
+        range = Range.new(10, Float::INFINITY, true)
+        expect(described_class.serialize(range)).to eq '[10,]'
+      end
+
+      it 'renders an unbounded lower as an empty side' do
+        range = Range.new(-Float::INFINITY, 101, true)
+        expect(described_class.serialize(range)).to eq '[,100]'
+      end
+
+      it 'renders a fully unbounded range as [,]' do
+        range = Range.new(-Float::INFINITY, Float::INFINITY, true)
+        expect(described_class.serialize(range)).to eq '[,]'
+      end
+
+      it 'handles negative bounds' do
+        range = Range.new(-10, 5, true)
+        expect(described_class.serialize(range)).to eq '[-10,4]'
+      end
+    end
+
+    context 'numeric ranges (numrange)' do
+      it 'formats inclusive BigDecimal bounds without scientific notation' do
+        range = Range.new(BigDecimal('5.5'), BigDecimal('7.0'), false)
+        expect(described_class.serialize(range)).to eq '[5.5,7]'
+      end
+
+      it 'strips a trailing .0 from a whole-number BigDecimal bound' do
+        range = Range.new(BigDecimal('0.0'), BigDecimal('14.0'), false)
+        expect(described_class.serialize(range)).to eq '[0,14]'
+      end
+
+      it 'does not touch a fractional bound that does not end in .0' do
+        range = Range.new(BigDecimal('1.25'), BigDecimal('3.75'), false)
+        expect(described_class.serialize(range)).to eq '[1.25,3.75]'
+      end
+
+      it 'preserves an exclusive upper bound (rare, only via raw data) as a paren form' do
+        range = Range.new(BigDecimal('1.0'), BigDecimal('2.0'), true)
+        expect(described_class.serialize(range)).to eq '[1,2)'
+      end
+
+      it 'renders a fully unbounded numeric range as [,]' do
+        range = Range.new(-Float::INFINITY, Float::INFINITY, true)
+        expect(described_class.serialize(range)).to eq '[,]'
+      end
+
+      it 'never emits BigDecimal engineering notation for the bounds' do
+        # BigDecimal#inspect (and, on some bigdecimal versions, plain #to_s)
+        # renders engineering notation, e.g. BigDecimal('5.5').inspect ==
+        # "0.55e1". That is not a valid range-literal bound, so the
+        # serializer must always go through #to_s('F'). Guard against a
+        # future edit accidentally swapping back to plain #to_s/#inspect.
+        expect(BigDecimal('5.5').inspect).to include 'e'
+        range = Range.new(BigDecimal('5.5'), BigDecimal('7.0'), false)
+        expect(described_class.serialize(range)).not_to include 'e'
+      end
+    end
+  end
+end