FEATURE: allow User Custom Field input field to accept the UserField name

[scossar]

Currently the add_user_to_group_through_custom_field script expects the UserCustomField name to be used when the automation is triggered with the "Recurring" trigger, and the UserField name to be used when the automation is triggered by the "User first logged in" trigger. This is confusing for users. For both triggers, it would be better if the UserField name could be used. That's the name that is displayed on the Admin / Customize / User Fields page.

This PR attempts to fix the issue in a backwards compatible way, so that existing automations that are making use of the UserCustomField name (for example: 'user_field_5') will not be broken.

The PR also updates the "with one matching user" test to set the UserCustomField name in the way that Discourse sets the name - based on the UserField's id.

Related discussion on Meta: https://meta.discourse.org/t/add-user-to-group-script-has-different-field-input-for-the-two-triggers/281813

[SamSaffron]

the test though feels incomplete, since it removes the old pattern?

[scossar]

The way I was reading the test is that it was saying that creating an automation with its "custom_field_name" set to "groupity_group" would successfully add a user to a group when triggered with the recurring trigger. So the test made it seem that the "custom_field_name" could be set to the user field name when an automation is triggered with the recurring trigger. But it can't, because a UserCustomField won't have its name set like this:

      UserCustomField.create!(
        user_id: user_1.id,
        name: "groupity_group",
        value: target_group.full_name,
      )
    end

instead, it will be set to user_field_#{user_field.id} depending on the UserField it's set from.

I don't mind making changes to that test if it's not clear.

Also, looking at this again, instead of using 'user_field_' in the updated query, it should use a :prefix placeholder (prefix: ::User::USER_FIELD_PREFIX). I can update the query so that it uses the same pattern of setting placeholders as the "USER_FIRST_LOGGED_IN" query: https://github.com/discourse/discourse-automation/blob/main/lib/discourse_automation/scripts/add_user_to_group_through_custom_field.rb#L64-L66

[scossar]

I've updated the PR to add a test for the case where the automation's custom_field_name is set to a UserCustomField name (for example user_field_1.

[lis2]

Hey @scossar,

Thank you for spotting inconsistency and this PR. Accepting both values solves the issue, but also it leaves inconsistent data.

I thought that it might be better to use that opportunity to unify data. In addition, UX was changed to allow admin to select a custom field. That should reduce confusion and chances for typo.

My PR with demo video is here:
#229

[scossar]

Perfect! A similar input type would be great for the public custom user fields and staff user custom fields site settings. There's a topic on Meta related to those settings: https://meta.discourse.org/t/adding-non-visible-user-custom-fields-to-the-api/276618.