fix(datasource-active-record): mark non-polymorphic has_one :through identity joins read-only

packages/forest_admin_agent/lib/forest_admin_agent/routes/resources/related/associate_related.rb

@@ -28,6 +28,7 @@ def handle_request(args = {})
             target_primary_key_values = Utils::Id.unpack_id(context.child_collection, args[:params]['data'][0]['id'],
                                                             with_key: true)
             relation = Schema.get_to_many_relation(context.collection, args[:params]['relation_name'])
+            Collection.assert_writable_relation!(args[:params]['relation_name'], relation)
 
             case relation.type
             when 'OneToMany'

packages/forest_admin_agent/lib/forest_admin_agent/routes/resources/related/dissociate_related.rb

@@ -34,6 +34,7 @@ def handle_request(args = {})
 
             filter = get_base_foreign_filter(args, context)
             relation = Schema.get_to_many_relation(context.collection, args[:params]['relation_name'])
+            Collection.assert_writable_relation!(args[:params]['relation_name'], relation)
 
             relation_name = args[:params]['relation_name']
             options = {

packages/forest_admin_agent/lib/forest_admin_agent/routes/resources/related/update_related.rb

@@ -28,6 +28,11 @@ def handle_request(args = {})
             # Must run before unpack_id: unauthorized callers get a 403, not a validation error
             context.permissions.can?(:edit, mutated_collection(relation, context))
 
+            # Every relation type this route writes through can be read-only (#379's
+            # OneToOne identity join is the reason this exists, but the flag itself sits on
+            # the shared RelationSchema, so enforce it once here rather than per-branch).
+            Collection.assert_writable_relation!(args[:params]['relation_name'], relation)
+
             parent_primary_key_values = Utils::Id.unpack_id(context.collection, args[:params]['id'])
 
             linked_primary_key_values = if (id = args.dig(:params, 'data', 'id'))

packages/forest_admin_agent/lib/forest_admin_agent/routes/resources/store.rb

@@ -67,6 +67,8 @@ def linked_one_to_one_relation(field, value, context)
           id = value.dig('data', 'id')
           return if id.nil?
 
+          ForestAdminDatasourceToolkit::Utils::Collection.assert_writable_relation!(field, schema)
+
           {
             schema: schema,
             foreign_collection: context.datasource.get_collection(schema.foreign_collection),

packages/forest_admin_agent/lib/forest_admin_agent/utils/schema/generator_field.rb

@@ -127,7 +127,7 @@ def build_one_to_one_schema(relation, collection, foreign_collection, base_schem
                 isFilterable: foreign_collection_filterable?(foreign_collection),
                 isPrimaryKey: false,
                 isRequired: false,
-                isReadOnly: key_field.is_read_only,
+                isReadOnly: relation.is_read_only || key_field.is_read_only,
                 isSortable: target_field.is_sortable,
                 validations: [],
                 reference: "#{foreign_collection.name}.#{relation.origin_key_target}"

packages/forest_admin_agent/spec/lib/forest_admin_agent/routes/resources/related/associate_related_spec.rb

@@ -81,6 +81,15 @@ module Related
                     origin_key_target: 'id',
                     origin_type_field: 'addressable_type',
                     origin_type_value: 'user'
+                  ),
+                  'locked_addresses' => Relations::ManyToManySchema.new(
+                    foreign_key: 'address_id',
+                    foreign_collection: 'address',
+                    foreign_key_target: 'id',
+                    through_collection: 'address_user',
+                    origin_key: 'user_id',
+                    origin_key_target: 'id',
+                    is_read_only: true
                   )
                 }
               }
@@ -191,6 +200,19 @@ module Related
             end
           end
 
+          it 'refuses to associate a read-only relation (any type, not just OneToOne, can be ' \
+             'marked is_read_only now that it lives on the shared RelationSchema)' do
+            args[:params]['relation_name'] = 'locked_addresses'
+            args[:params]['data'] = [{ 'id' => 1 }]
+            args[:params]['id'] = 1
+            allow(@datasource.get_collection('address_user')).to receive(:create)
+
+            expect { associate.handle_request(args) }
+              .to raise_error(ForestAdminDatasourceToolkit::Exceptions::ValidationError,
+                              'Field locked_addresses is not editable')
+            expect(@datasource.get_collection('address_user')).not_to have_received(:create)
+          end
+
           context 'when call on one to many relation' do
             before do
               args[:params]['relation_name'] = 'address_users'

packages/forest_admin_agent/spec/lib/forest_admin_agent/routes/resources/related/dissociate_related_spec.rb

@@ -41,6 +41,15 @@ module Related
                   origin_key_target: 'id',
                   origin_type_field: 'addressable_type',
                   origin_type_value: 'user'
+                ),
+                'locked_addresses' => Relations::ManyToManySchema.new(
+                  foreign_key: 'address_id',
+                  foreign_collection: 'address',
+                  foreign_key_target: 'id',
+                  through_collection: 'address_user',
+                  origin_key: 'user_id',
+                  origin_key_target: 'id',
+                  is_read_only: true
                 )
               }
             )
@@ -240,6 +249,20 @@ module Related
               )
             end
 
+            it 'refuses to dissociate a read-only relation (any type, not just OneToOne, can be ' \
+               'marked is_read_only now that it lives on the shared RelationSchema)' do
+              allow(@datasource.get_collection('address_user')).to receive(:delete)
+
+              args[:params]['relation_name'] = 'locked_addresses'
+              args[:params][:data] = [{ 'id' => 1 }]
+              args[:params]['id'] = 1
+
+              expect { dissociate.handle_request(args) }
+                .to raise_error(ForestAdminDatasourceToolkit::Exceptions::ValidationError,
+                                'Field locked_addresses is not editable')
+              expect(@datasource.get_collection('address_user')).not_to have_received(:delete)
+            end
+
             it 'call dissociate_or_delete_many_to_many without deletion' do
               allow(@datasource.get_collection('address_user'))
                 .to receive_messages(list: [AddressUser.new(1, 1, 1)], delete: true)

packages/forest_admin_agent/spec/lib/forest_admin_agent/routes/resources/related/update_related_spec.rb

@@ -27,6 +27,12 @@ module Related
                   origin_key_target: 'id',
                   foreign_collection: 'book'
                 ),
+                'locked_book' => Relations::OneToOneSchema.new(
+                  origin_key: 'author_id',
+                  origin_key_target: 'id',
+                  foreign_collection: 'book',
+                  is_read_only: true
+                ),
                 'address' => Relations::PolymorphicOneToOneSchema.new(
                   origin_key: 'addressable_id',
                   foreign_collection: 'address',
@@ -47,6 +53,12 @@ module Related
                   foreign_key: 'author_id',
                   foreign_key_target: 'id',
                   foreign_collection: 'user'
+                ),
+                'locked_author' => Relations::ManyToOneSchema.new(
+                  foreign_key: 'author_id',
+                  foreign_key_target: 'id',
+                  foreign_collection: 'user',
+                  is_read_only: true
                 )
               }
             )
@@ -157,6 +169,21 @@ module Related
               expect(result).to eq({ content: nil, status: 204 })
             end
 
+            it 'refuses to write a read-only many_to_one relation (not just OneToOne can be ' \
+               'marked is_read_only now that it lives on the shared RelationSchema)' do
+              allow(@datasource.get_collection('book')).to receive(:update)
+
+              args[:params]['collection_name'] = 'book'
+              args[:params]['relation_name'] = 'locked_author'
+              args[:params]['data'] = { 'id' => 1 }
+              args[:params]['id'] = 1
+
+              expect { update.handle_request(args) }
+                .to raise_error(ForestAdminDatasourceToolkit::Exceptions::ValidationError,
+                                'Field locked_author is not editable')
+              expect(@datasource.get_collection('book')).not_to have_received(:update)
+            end
+
             it 'call handle_request on a polymorphic_many_to_one relation' do
               allow(permissions).to receive(:get_scope)
                 .and_return(Nodes::ConditionTreeLeaf.new('location', Operators::EQUAL, 'paris'))
@@ -277,6 +304,21 @@ module Related
               expect(result).to eq({ content: nil, status: 204 })
             end
 
+            it 'refuses to write a read-only one_to_one relation (#379: it would overwrite ' \
+               "the foreign collection's own primary key)" do
+              allow(@datasource.get_collection('book')).to receive_messages(aggregate: [{ 'value' => 1 }], update: true)
+
+              args[:params]['collection_name'] = 'user'
+              args[:params]['relation_name'] = 'locked_book'
+              args[:params]['data'] = { 'id' => 1 }
+              args[:params]['id'] = 1
+
+              expect { update.handle_request(args) }
+                .to raise_error(ForestAdminDatasourceToolkit::Exceptions::ValidationError,
+                                'Field locked_book is not editable')
+              expect(@datasource.get_collection('book')).not_to have_received(:update)
+            end
+
             it 'call handle_request on a polymorphic_one_to_one relation' do
               allow(permissions).to receive(:get_scope)
                 .and_return(Nodes::ConditionTreeLeaf.new('location', Operators::EQUAL, 'paris'))

packages/forest_admin_agent/spec/lib/forest_admin_agent/routes/resources/store_spec.rb

@@ -408,6 +408,51 @@ def respond_to?(arg)
             end
           end
 
+          describe 'with a read-only one to one relation' do
+            before do
+              @datasource.get_collection('person').schema[:fields]['locked_passport'] =
+                Relations::OneToOneSchema.new(
+                  origin_key: 'person_id',
+                  origin_key_target: 'id',
+                  foreign_collection: 'passport',
+                  is_read_only: true
+                )
+            end
+
+            it 'refuses to link it (#379: it would overwrite the foreign collection\'s own primary key)' do
+              args[:params][:data] = {
+                attributes: { 'name' => 'john' },
+                relationships: { 'locked_passport' => { 'data' => { 'type' => 'passports', 'id' => 1 } } },
+                type: 'persons'
+              }
+              args[:params]['collection_name'] = 'person'
+              allow(@datasource.get_collection('person')).to receive(:create)
+              allow(@datasource.get_collection('passport')).to receive(:update)
+
+              expect { store.handle_request(args) }
+                .to raise_error(ForestAdminDatasourceToolkit::Exceptions::ValidationError,
+                                'Field locked_passport is not editable')
+              expect(@datasource.get_collection('person')).not_to have_received(:create)
+              expect(@datasource.get_collection('passport')).not_to have_received(:update)
+            end
+
+            it 'ignores a read-only one to one relationship carrying no data, same as a writable one' do
+              args[:params][:data] = {
+                attributes: { 'name' => 'john' },
+                relationships: { 'locked_passport' => { 'data' => nil } },
+                type: 'persons'
+              }
+              args[:params]['collection_name'] = 'person'
+              allow(@datasource.get_collection('person')).to receive_messages(
+                create: { 'id' => 1, 'name' => 'john' },
+                list: [{ 'id' => 1, 'name' => 'john' }]
+              )
+
+              expect { store.handle_request(args) }.not_to raise_error
+              expect(@datasource.get_collection('passport')).not_to have_received(:update)
+            end
+          end
+
           describe 'with polymorphic one to one relation' do
             before do
               collection_address = build_collection(

packages/forest_admin_agent/spec/lib/forest_admin_agent/utils/schema/generator_field_one_to_one_spec.rb

@@ -63,6 +63,36 @@ module Schema
             )
           end
 
+          it 'marks the field read-only when the relation itself is read-only, even if the ' \
+             'underlying key column is writable' do
+            collection_note = Collection.new(@datasource, 'Note')
+            collection_note.add_fields(
+              {
+                'id' => ColumnSchema.new(column_type: 'Number', is_primary_key: true, is_read_only: false),
+                'author_id' => ColumnSchema.new(column_type: 'String', is_read_only: false, is_sortable: true)
+              }
+            )
+
+            collection_person = Collection.new(@datasource, 'PersonReadonly')
+            collection_person.add_fields(
+              {
+                'id' => ColumnSchema.new(column_type: 'Number', is_primary_key: true),
+                'note' => Relations::OneToOneSchema.new(
+                  origin_key: 'author_id',
+                  origin_key_target: 'id',
+                  foreign_collection: 'Note',
+                  is_read_only: true
+                )
+              }
+            )
+            @datasource.add_collection(collection_note)
+            @datasource.add_collection(collection_person)
+
+            schema = described_class.build_schema(@datasource.get_collection('PersonReadonly'), 'note')
+
+            expect(schema[:isReadOnly]).to be true
+          end
+
           it 'generate inverse relation' do
             schema = described_class.build_schema(@datasource.get_collection('Book'), 'author')
 

packages/forest_admin_datasource_active_record/lib/forest_admin_datasource_active_record/collection.rb

@@ -114,6 +114,15 @@ def valid_many_to_many_source?(association)
       association.source_reflection&.belongs_to?
     end
 
+    # Same criterion as unrepresentable_many_to_many?: a composite primary key can arrive as a
+    # real Array or already flattened by Rails into a mangled String -- checking column
+    # membership catches both forms, plus a custom primary_key pointing at a non-column, all of
+    # which would otherwise hit nil.column_type in build_one_to_one_schema (#379).
+    def unrepresentable_one_to_one?(association)
+      !association.klass.column_names.include?(association.klass.primary_key) ||
+        !@model.column_names.include?(@model.primary_key)
+    end
+
     def build_many_to_many_field(association, through_reflection, is_polymorphic, source_polymorphic)
       ForestAdminDatasourceToolkit::Schema::Relations::ManyToManySchema.new(
         foreign_collection: format_model_name(association.klass.name),
@@ -157,15 +166,19 @@ def fetch_associations
 
               if many_to_many_shape
                 add_many_to_many_field(association, through_reflection, is_polymorphic, source_polymorphic)
+              elsif unrepresentable_one_to_one?(association)
+                warn_unrepresentable_one_to_one(association, through_reflection)
               else
                 add_field(
                   association.name.to_s,
                   ForestAdminDatasourceToolkit::Schema::Relations::OneToOneSchema.new(
                     foreign_collection: format_model_name(association.klass.name),
                     origin_key: association.klass.primary_key,
-                    origin_key_target: @model.primary_key
+                    origin_key_target: @model.primary_key,
+                    is_read_only: true
                   )
                 )
+                warn_readonly_identity_join(association, through_reflection)
               end
             elsif association.inverse_of&.polymorphic?
               add_field(
@@ -362,6 +375,33 @@ def warn_identity_join(association, through_reflection)
       )
     end
 
+    def warn_unrepresentable_one_to_one(association, through_reflection)
+      logger = ActiveSupport::Logger.new($stdout)
+      logger.warn(
+        "[ForestAdmin] ⚠️  Skipping association '#{association.name}' in model '#{@model.name}': " \
+        "this has_one :through (via '#{format_model_name(through_reflection.klass.name)}') can't be " \
+        'represented as a Forest Admin one-to-one -- one of its endpoints has a composite primary key.'
+      )
+    end
+
+    # OneToOneSchema has no through_collection, so this publishes each side's own primary key
+    # as a placeholder join -- the columns exist on both sides, so GeneratorField's field
+    # lookups don't raise, but the join isn't meaningful data (it matches two unrelated
+    # sequences), which is exactly why it's marked read-only rather than left writable through
+    # what's really the foreign collection's own primary key (#379).
+    def warn_readonly_identity_join(association, through_reflection)
+      logger = ActiveSupport::Logger.new($stdout)
+      foreign_key = association.klass.primary_key
+      origin_key = @model.primary_key
+      logger.warn(
+        "[ForestAdmin] ⚠️  Association '#{association.name}' in model '#{@model.name}' is published " \
+        "as a read-only one-to-one joining '#{format_model_name(association.klass.name)}'.'#{foreign_key}' " \
+        "to this model's own '#{origin_key}': it's a has_one chained through " \
+        "'#{format_model_name(through_reflection.klass.name)}', which OneToOneSchema can't express as a " \
+        'real two-hop join -- see #379 for background.'
+      )
+    end
+
     def warn_missing_polymorphic_columns(association)
       missing_columns = []
       missing_columns << association.foreign_key unless schema[:fields][association.foreign_key]