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.

Sign up for GitHub

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

Jump to bottom

Add labels to membership type metadata, allowing for addField method to be used #12132

Merged
merged 3 commits into from
May 15, 2018
Merged

Add labels to membership type metadata, allowing for addField method to be used #12132

merged 3 commits into from
May 15, 2018

Conversation

eileenmcnaughton
Copy link
Contributor

@eileenmcnaughton eileenmcnaughton commented May 15, 2018 •
edited
Loading

Overview

As a follow on from #12124 this adds a metadata option to specify the html label and alters some fields to use it.

Before

No change

After

No change. Coding methodology for metadata flexibility on labels established
(I will update docs once merged)

Technical Details

If we switch fields to use the (recommended) addField the labels can sometimes change. Arguably we should update the existing 'title field' in the xml to reflect the desired html label. However, these labels tend to have information about the entity - possibly intended to disambiguate Membership Type.is_active from Option_group.is_active in a flattish array - perhaps in views or the api explorer. Introducing an override label option specific to the html representation saves us (for better & worse) from having to fully explore these computing uses.

Comments

@colemanw this is per your suggestion. It keeps the labels the same (although arguably in some cases not for the better). Looking at the form I still see that description could be taken from the metadata & that might be a next step - ie.

name.description in the tpl "e.g. 'Student', 'Senior', 'Honor Society'..."
name.description in the xml - none

description.description in the xml Description of Membership Type
description.description in the tpl 'Description of this membership type for internal use. May include eligibility, benefits, terms, etc.'

In both those cases I would be inclined to update the xml with the tpl description. However, I presume there would be cases where it makes sense to be able to define

<html>description>blah</description></html>

The other notable thing in the form that could be defined by metadata is the rules - ie.

    $this->addRule('name', ts('A membership type with this name already exists. Please select another name.'),
      'objectExists', array('CRM_Member_DAO_MembershipType', $this->_id)
    );
    $this->addRule('minimum_fee', ts('Please enter a monetary value for the Minimum Fee.'), 'money');

I wondered about adding a tag - either within or as a sister to

@colemanw
Copy link
Member

I think it ought to get wrapped in ts() in the DAO file, right?

@colemanw
Copy link
Member

@civicrm-builder retest this please.

@totten
Copy link
Member

totten commented May 15, 2018

jenkins, test this please

@eileenmcnaughton
Copy link
Contributor Author

ah yes @colemanw you are correct. Do you think this is merge on pass now?

@eileenmcnaughton eileenmcnaughton merged commit 48f42a2 into civicrm:master May 15, 2018
@eileenmcnaughton eileenmcnaughton deleted the membership_type_data branch May 15, 2018 21:03
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers
No reviews
Assignees
No one assigned
Labels
master merge on pass
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
4 participants
@eileenmcnaughton @colemanw @totten @civicrm-builder