Commit graph

7 commits

Author SHA1 Message Date
Daniel Uber
1a07ad8d9e
Profile attribute names should be unique and non-empty (#16396)
* transliterate when generating attribute names to avoid emptiness

If you enter a non-ascii (non `\w` matching) string as the profile
field label, the attribute name is the empty string.

This causes problems outlined in #16391

To avoid losing user provided data, transliterate (Sterile is the same
tool we're using for Article#title_to_slug) before matching against
the word regex.

If we persist an empty string, or persist a non-`\w` name, the
coordinating regex in Profile::ATTRIBUTE_NAME_REGEX will leave a nil
match and raise NoMethodError (the method missing is for
match[:attribute_name] when match was nil, not Profile - that's the
next commit.

* Guard against nil matches

If an attribute name doesn't match the regex, the match is nil, and
trying to access (nil)[:attribute_name] raises a NoMethodError.

If there was no match, assume profile does not respond to the
selector, and don't handle it in method missing.

* Ensure generated attribute name is valid before saving

This raises a validation error if the generated name (from the label)
would be empty.

It's not an optimal error message (since the user can't see the
internal attribute name) but it prevents persisting broken/empty data

* actually raise error when validating

validate/valid? only return true or false (and set errors on the
object). In order to reject the creation, we need to raise a
validation error, not only call validate. I think this is because the
execution of before_create hooks happens after validation (which is
why the validation was only checked on update, not create).

* Generate an attribute name completely independent of the field label

This prevents mistakenly labeling a field "Class" or "Association" or
one of the other hundred public methods an AR model like Profile
exposes. Since attribute_name will be passed to `profile.public_send`
we really shouldn't build selectors from user supplied inputs.

* Use the admin supplied label in the sidebar

The profile decorator is used in the Users#show page to populate the
sidebar fields. Don't use the attribute name (which we mangled during
creation, and now generate randomly) as the label, use the label.

* Make the label lookup null safe, and filter attributes more

* Update data update script to not expect predictable labels

This is low impact since it ran in july, but we no longer know what
attribute name a label will create.

* Fix moderator spec

"Test Field" label no longer predictably generates :test_field as an
attribute name. Ask the field what it's name is before asking profile
about it.

* Fix profile preview card request spec

Remove the assumption that profile responds to a method name based on
the label for Work and Education fields.

* Update old DUS and its test

This can probably be archived

* Update profile spec to use fields generated attribute names

We used to "know" how attributes were generated from labels. Now we don't.

* update e2e seeds for profile field change

* Don't expect attribute name to be based on the label

* expect created profile fields respond to their attribute name

* Update system test

The label, not the attribute name, is shown on the profile form (the
field has an id related to the attribute name, but the view shows the
mutable/human-readable label).

* Keep the field title lowercase when sending the json preview card

The userMetadata component expects "work" and "education" to be
attributes of the metadata, but the ui_attributes_for() method was
titlizing these (for display).

Ideally we wouldn't have "special purposed" these two field names, but
they're there.

* update profile field by attribute name, not based on label

* Update profile field removal assumptions

We don't know what the method selectors will be, we have to ask.

* remove old test

* Update translations for Education and Work

Since the ui_attributes_for(area:) now gives the label, not the
attribute, we need to match the label of the profile field.

Note to self: this exposes an issue in localizing the custom profile
fields (probably a bigger problem for large, international communities
like DEV than some others, but trying to match static translation
files against user-modifiable database records seems like a problem
we'll see again).

* Empty commit to retrigger buildkite

* Remove profile field migration update scripts

Cloned the specs from the other "remove unused scripts" script.

* Remove unused scripts

The data update script removes the entry from the table (recording
that these have run) - we also want to remove the files (preventing
them from running again).

* remove unneeded spec for removed file

* when translation for header area field not found, use the title

Only Work and Education already have keys in the yml translation file,
and there's not a great (or easy?) way to make multiple translations
on these fields right now.

Since an admin can create a new field, and assign it to the header
area, we can't assume the code has a configured translation key for
this field.

Fallback to the title (we do this in another context already) if
there's no translation.

* PR feedback: Avoid n+1 query for labels

the original implementation of "label_for_attribute" had an n+1 query
looping over each matched key.

Follow suggested improvement and pull labels and attributes at once
from the db and modify the returned hash.

* Downcase title before looking for translation key

This avoids putting "odd" capitalized keys into the yml translation
file

Revert addition of "Work" and "Education" to the users files.

* use a let binding for duplicated test data

* Update app/models/profile_field.rb

prefer SecureRandom.hex for a dashless uuid (instead of removing the dashes).

Co-authored-by: Jamie Gaskins <jgaskins@hey.com>

Co-authored-by: Jamie Gaskins <jgaskins@hey.com>
2022-04-04 12:14:02 -05:00
Daniel Uber
afa214ac52
Require profile fields to belong to a group (#16390)
* Require profile fields to belong to a group

Remove the empty group dropdown from the group select tag

Remove the optional: true from the belongs_to validation (raise a
validation error on save if the group is not provided)

Update test to remove belongs_to's optional setting

* Update ProfileFields::Add spec to include profile field group

* update factory to ensure a non-empty profile field group

* Add required profile field group to seeded profile fields

Since profile field is required, create was failing (silently).

Add required field so these are available in e2e tests.

* Use factory `association` to create a profile field group

https://github.com/thoughtbot/factory_bot/blob/master/GETTING_STARTED.md#explicit-definition

* provide required profile field group when posting data

* Fix one spec, drop another

Both of these scripts will be removed in the other branch (changing
the attribute name) - dropping the test here seems lower trouble than
modifying an out of data data update script to pass.

* skip test slated for removal

Similar to the last commit - this test exercises a script about to be
archived in the other branch.
2022-02-15 09:27:18 -06:00
Michael Kohl
f11e7b49d4
Remove check_box type from profile fields (#14832)
* Remove check_box type from profile fields

* Fix profile field spec

* Delete emphasized_comments_experiment.rb

This file was not supposed to be part of this PR
2021-09-30 13:27:39 -04:00
Michael Kohl
cfe288d8cc
[deploy] Start ignoring group column for profile fields (#10045)
* Start ignoring group column for profile fields

* Fix specs

Co-authored-by: mstruve <mollylbs@gmail.com>
2020-08-31 10:16:37 +07:00
Michael Kohl
161ed7d55e
[deploy] Introduce ProfileFieldGroup model (#10007)
* Introduce ProfileFieldGroup model

* Make profile_fields_groups name column unique

* Fix some specs

* feat: allow the page to work again

* Add guard clause to data update script

* Remove unused file

* Fix specs

* Add foreign key

Co-authored-by: Ridhwana <ridhwana.khan16@gmail.com>
2020-08-28 10:51:33 +07:00
Ridhwana
b0bacf60fb
Profile Fields Admin UI v2 (#9830)
* feat: add a select and a new text field

* feat: groups need to be unique and present

* feat: make the ui a little better

* chore: update the label

* chore: just present

* feat: add some js ( still a work in progress)

* feat: update the interface to work better :)

* chore: indent

* feat: make sure the toggles work

* chore: rename more aptly

* refactor: toggling pulled out into two functions

* refactor: move group form to a partial so that we can reuse it

* chore: move the js to the group form

* chore: update alignment and nav element

* chore: update text

* chore: update specs
2020-08-25 17:39:12 +02:00
Michael Kohl
b3e7384e24
[deploy] Automatically refresh profile accessors (#9843)
* Rename method

* Add ProfileFields::Add service

* Add ProfileFields::Remove service

* Use remove_method instead of undef_method

* All hail undef_method
2020-08-19 21:25:59 -04:00