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

Show disabled membership types on contact tab #19431

Merged

Conversation

mattwire
Copy link
Contributor

Overview

Replacement for #17143 / #17435 using API4 and retaining financialacl code.

Contacts may still have disabled membership types. But new memberships of that type cannot be created.

Before

Disabled memberships show in searches but not contact tab.

After

Disabled memberships show in searches and contact tab.

Technical Details

Use API4 MembershipType API to retrieve all membership types that the user has permission for. Added a comment that the code should be moved to the financialacl extension instead.
Set checkPermissions explicitly to TRUE (that is the default) because this allows the API to accurately return the membership types that the user has permission to access. For example another extension could control access.

Comments

@seamuslee001

@civibot
Copy link

civibot bot commented Jan 21, 2021

(Standard links)

@civibot civibot bot added the master label Jan 21, 2021
@seamuslee001
Copy link
Contributor

Bunch of test failures here @mattwire the code looks the right direction here @eileenmcnaughton I'm just thinking about the implications in the changes to CRM_Utils_Money::format here

CRM/Utils/Money.php Outdated Show resolved Hide resolved
@mattwire mattwire force-pushed the membershiptypedisabledcontacttab branch from bc99c6a to 2853d3e Compare January 21, 2021 21:15
@mattwire
Copy link
Contributor Author

@seamuslee001 Oh sorry I've done it again. Those commits should not have been in this PR - now updated.

@mattwire mattwire force-pushed the membershiptypedisabledcontacttab branch 2 times, most recently from 165e4a0 to 2833d31 Compare January 28, 2021 12:31
@seamuslee001
Copy link
Contributor

This change looks right to me now, I just have one minor r-code issue but I don't see any other issues with it @eileenmcnaughton

@eileenmcnaughton
Copy link
Contributor

Yep I think this is mergeable with the switch @seamuslee001 suggested

@mattwire mattwire force-pushed the membershiptypedisabledcontacttab branch from 2833d31 to 2dc76e8 Compare January 29, 2021 09:44
@civicrm civicrm deleted a comment from eileenmcnaughton Jan 31, 2021
@mattwire
Copy link
Contributor Author

@eileenmcnaughton @seamuslee001 Thanks, tests now passing

@eileenmcnaughton
Copy link
Contributor

Fix for regression caused by this #19594

@mattwire mattwire deleted the membershiptypedisabledcontacttab branch March 4, 2021 09:08
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants