Shouldn't we also add some kind of backend validation?

@oriolgual I'm having problems making the validation in:
decidim-proposals/lib/decidim/proposals/component.rb

It turns out that instance.reload does not work in this case to find out if participatory_text_enabled was different before the update:

instance.settings.participatory_texts_enabled == instance.reload.settings.participatory_texts_enabled

And the solutions I've come up are not compatible with the specs:

• Reconnecting with the database

  component.on(:update) do |instance|
    Decidim::Component.connection.reconnect! # instance.reload is not enough to get the db's setting value
    db_setting_value = Decidim::Component.find(instance.id).settings.participatory_texts_enabled
    next if db_setting_value == instance.settings.participatory_texts_enabled
    raise "Can't update ParticipatoryText setting when there are proposals" if Decidim::Proposals::Proposal.where(component: instance).any?
  end

PG::NoActiveSqlTransaction: ERROR:  ROLLBACK TO SAVEPOINT can only be used in transaction blocks
       : ROLLBACK TO SAVEPOINT active_record_1

• Using PaperTrail versions

  component.on(:update) do |instance|
    return unless (settings_changeset = instance.versions.last.changeset["settings"])
    before = settings_changeset.first["global"]["participatory_texts_enabled"]
    after = settings_changeset.last["global"]["participatory_texts_enabled"]
    raise "Can't update ParticipatoryText setting when there are proposals" if before != after && Decidim::Proposals::Proposal.where(component: instance).any?
  end

     Failure/Error: return unless (settings_changeset = instance.versions.last.changeset["settings"])
     
     NoMethodError:
       undefined method `changeset' for nil:NilClass

The second could be fixed by creating PaperTrail versions in the factories, but I don't know if it's a good choice as I would need to update many specs...

Would it be acceptable to move the validation for to the UpdateComponent command inside decidim-core?

Would it be acceptable to move the validation for to the UpdateComponent command inside decidim-core?

Yes, that seems OK!

Is everything finished here @aitorlb ?

@oriolgual I was just about to push a little refactor if that is okay with you:

      def must_be_able_to_change_participatory_texts_setting
        return unless settings[:participatory_texts_enabled] &.!= component.settings.participatory_texts_enabled

        errors.add(:settings) if Decidim::Proposals::Proposal.where(component: component).any?
      end

I just found out that you can use the &. operator with comparisons...

@oriolgual I was just about to push a little refactor if that is okay with you:

      def must_be_able_to_change_participatory_texts_setting
        return unless settings[:participatory_texts_enabled] &.!= component.settings.participatory_texts_enabled

        errors.add(:settings) if Decidim::Proposals::Proposal.where(component: component).any?
      end

I just found out that you can use the &. operator with comparisons...

LOL, what does &. do when comparing?

Oh, I guess it doesn't crash if settings is nil, nice!

It would be equivalent to the following:

settings[:participatory_texts_enabled] && settings[:participatory_texts_enabled] != component.settings.participatory_texts_enabled

It's just a little shorter, hehe...

Oh, I guess it doesn't crash if settings is nil, nice!

The problem is not that it crashes but that it gives a false positive:
if settings[:participatory_texts_enabled] returns nil, it gets compared to component.settings.participatory_texts_enabled. We DON'T want that.