-
Notifications
You must be signed in to change notification settings - Fork 167
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
Issue #3476285: Improve usability of Taxonomy term creations #4139
Issue #3476285: Improve usability of Taxonomy term creations #4139
Conversation
a74b63e
to
e6f62ad
Compare
...cial_profile/modules/social_profile_organization_tag/social_profile_organization_tag.install
Outdated
Show resolved
Hide resolved
...cial_profile/modules/social_profile_organization_tag/social_profile_organization_tag.install
Outdated
Show resolved
Hide resolved
/** | ||
* Create entity-form-display or hidden path field from Profile Tag taxonomy. | ||
*/ | ||
function social_profile_update_130003(): void { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
function social_profile_update_130003
and social_profile_update_130004
and social_profile_update_130005
are very duplicated code to me.
Can we create a service to use instead of copy pasting?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Ideally social_content_report_update_130002
would be using the service as well if social_profile and social_content_report share a module dependency.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I created a function to avoid duplicate hook-update with same code, but I done only for Social Profile, because create a Service would complex because in Cablecar isn't be possible to use before a release.
These hook-update will be deleted in a near future, but let me know if you have other idea.
/** | ||
* Create entity-form-display or hide path field from Organization Tag taxonomy. | ||
*/ | ||
function social_profile_organization_tag_update_130002(): void { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Can you explain why this update needs to be done using update hook and not form_alter?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The Acceptance Criteria is asking to change using configuration, because that way can reverted easily by other system if necessary.
c174318
to
ef7ddc8
Compare
Some taxonomies doesn't have entity-form-display created, so a logic is created to check it, after this validation the entity-form-display is created or updated.
9a6155f
to
7933259
Compare
Problem (for internal)
Currently we have several taxonomy vocabularies and we display default options to SM.
Some of them are not relevant and should be hidden.
Solution (for internal)
The fields are hidden using configuration, but don't config-modify or update-helper because some taxonomies doesn't have entity-form-display created.
So I created a logic to create entity-form-display when it doesn't exist or update when it already be created.
Release notes (to customers)
Improved the user-experience to create taxonomy-terms, some unnecessary fields was hide.
Issue tracker
PROD-30250
#3476285
Theme issue tracker
N/A
How to test
Taxonomies list:
Content tags:
-- URL field visible
Profile tag
-- URL field is not visible
Event type:
-- URL field is not visible
Report reason
-- URL field is not visible
Topic type
-- URL field is not visible
Nationality
-- URL field is not visible
Interest
-- URL field is not visible
Group type
-- URL field is not visible
Expertise
-- URL field is not visible
Organization
-- URL field is not visible
Change Record
N/A
Translations
N/A