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

CRM-17789 - CRM_Custom_Form_Group - Simplify reltype list #9035

Merged
merged 2 commits into from
Sep 14, 2016
Merged

CRM-17789 - CRM_Custom_Form_Group - Simplify reltype list #9035

merged 2 commits into from
Sep 14, 2016

Conversation

totten
Copy link
Member

@totten totten commented Sep 14, 2016 •
edited by civicrm-builder
Loading

@totten totten mentioned this pull request Sep 14, 2016
Closed
@totten totten force-pushed the master-group-cleanup branch from 62c4cc2 to a2e2fbe Compare September 14, 2016 03:37
@totten
CRM_Custom_Form_Group - -1 spaghetti
a9c1ff4 
So...  there's a helper function `getContactRelationshipType`
which provided a restricted listing of relationship-types (as they apply
to specific contact types or subtypes).

But what if you wanted the list of relationship-types, without filtering by
contact-type?  You *could* go through and query
eve.ry.sin.gle.contac.ttype.and.cont.act.subty.pe.  And then merge all of
these lists.  And then parse and remerge the strange intermediate format.

Or...  you just load the list you wanted...  directly...  in the format you
wanted...
@totten totten force-pushed the master-group-cleanup branch from a2e2fbe to a9c1ff4 Compare September 14, 2016 03:41
@totten
Copy link
Member Author

totten commented Sep 14, 2016

Score: -30 SLOC on the principal! (Alas, +18 on the simple/generic helper w/docs...)

@seamuslee001 seamuslee001 mentioned this pull request Sep 14, 2016
Closed
@seamuslee001
Copy link
Contributor

tested this, works sensibly

@jitendrapurohit
Copy link
Contributor

jitendrapurohit commented Sep 14, 2016 •
edited
Loading

Tested this using different relationship types(normal/multilingual setup) for subtypes, organisation, households, etc. All seems to be loading in the formatted display correctly. This is good to be merged from my side too.

Thanks!

@totten
Copy link
Member Author

totten commented Sep 14, 2016

jenkins, test this please

1 similar comment
@totten
Copy link
Member Author

totten commented Sep 14, 2016

jenkins, test this please

@totten
Copy link
Member Author

totten commented Sep 14, 2016

The reported failure CRM_Contribute_Form_ContributionTest.testSubmitCreditCardPayPal is pre-existing/not-relevant.

@totten totten merged commit ca9cc1e into civicrm:master Sep 14, 2016
@jitendrapurohit jitendrapurohit mentioned this pull request Oct 19, 2016
Merged
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
Projects
None yet
Milestone
No milestone
Development

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