Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

afform - Get default field <label> from label instead of title #18989

Merged
merged 2 commits into from
Nov 18, 2020

Conversation

totten
Copy link
Member

@totten totten commented Nov 18, 2020

Overview

This updates afform/core and afform/gui to use better default labels -- e.g. when you add the gender_id field, it should default to "Gender" (label) instead of "Gender ID" (title).

Before

  • In previous release. <Entity>.getFields returned a title. The title describes a field somewhat abstractly (e.g. gender_id has the title Gender ID).
  • In the Afform UI, this title was used as the default.
  • In the Afform GUI+HTML, you could optionally override the title.

After

  • In current releases, <Entity>.getfields returns both a title and label. The label is simpler / more end-user-y string (e.g. Gender vs Gender ID).
  • In the Afform UI, this label was used as the default.
  • In the Afform GUI+HTML, you could optionally override the label.

Comments

This is technically a breaking-change for anyone who has overridden the default title - they would have to re-enter it as a label override. Ideally, this would be transitioned for compatibility -- but we don't have the validation/migration mechanism yet. There's a reasonable argument that it's not important to have a migration for this field at the current stage of development. More discussion on Mattermost: https://chat.civicrm.org/civicrm/pl/btbh1n88tp88zx39ean8gtgjke

Note that the PR adds an item about the BC break to the auditor backlog, which has a list of things that should be included in validation/migration.

CC @colemanw @eileenmcnaughton

@civibot
Copy link

civibot bot commented Nov 18, 2020

(Standard links)

@civibot civibot bot added the master label Nov 18, 2020
@colemanw
Copy link
Member

@totten test failures relate

@totten totten force-pushed the master-afform-label branch from cbb1f91 to 44f7db4 Compare November 18, 2020 19:37
@totten
Copy link
Member Author

totten commented Nov 18, 2020

@colemanw Thanks, yup. Updated test to match schema change. Squashed/rebased.

@eileenmcnaughton eileenmcnaughton merged commit 7343a2d into civicrm:master Nov 18, 2020
@totten totten deleted the master-afform-label branch November 18, 2020 23:44
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants