Skip to content

CSTMM-118: Refresh Membership Count Upon Updating Relationships#554

Open
shahrukh-compuco wants to merge 1 commit into
masterfrom
csmm-118-fix-membership-count
Open

CSTMM-118: Refresh Membership Count Upon Updating Relationships#554
shahrukh-compuco wants to merge 1 commit into
masterfrom
csmm-118-fix-membership-count

Conversation

@shahrukh-compuco

@shahrukh-compuco shahrukh-compuco commented Mar 31, 2025

Copy link
Copy Markdown
Contributor

Overview

Currently when user updates or enable/disable any relationship the membership count does not get updated and it only gets updated when user clicks on membership tab

Before

screen_recording_before

After

The membership count gets updated as soon as the user updates any relationship
screen_recording_after

Technical Details

Currently when user changes any relationship civicrm core adds a soft reset to the membership and contribution tab i.e it will only be refreshed when user clicks on the tab but in this PR we change the membership tab to refresh immediately after a change to a relationship.

@jamienovick

jamienovick commented Mar 31, 2025

Copy link
Copy Markdown

@shahrukh-compuco
Can we add details of how this problem was fixed and why that approach was taken?

Some other questions...

  • Is this a problem in CiviCRM core. If so would a core fix be more appropriate?
  • Would this issue occur on other tabs? If so would a different fix be more flexible/generalised?
  • Are there edge cases that could break this fix. For example changing the tab order or names, other extensions that could affect this fix.
  • Does this fix give us additional code / manual tests that we need to perform in future?

Is this the most sustainable approach to solving this issue? If not are there strong reasons for not taking a more sustainable approach?

@shahrukh-compuco

shahrukh-compuco commented Mar 31, 2025

Copy link
Copy Markdown
Contributor Author

@shahrukh-compuco Can we add details of how this problem was fixed and why that approach was taken?

Some other questions...

  • Is this a problem in CiviCRM core. If so would a core fix be more appropriate?
  • Would this issue occur on other tabs? If so would a different fix be more flexible/generalised?
  • Are there edge cases that could break this fix. For example changing the tab order or names, other extensions that could affect this fix.
  • Does this fix give us additional code / manual tests that we need to perform in future?

Is this the most sustainable approach to solving this issue? If not are there strong reasons for not taking a more sustainable approach?

@jamienovick I have added the technical details section to the PR also along with that this issue does not exist and its working fine on core as in latest version of civi the display has quiet changed in terms of code as now civi is using search kit to display memberships on membership tab also regarding the breaking chances its the code that was being used in civicrm core with just a slight modification that we refresh the membership tab as soon as any relationship gets updated but still I would mention these cases to test in ticket and for and for future tests I think it would be ok only to test the issue mentioned in ticket

@jamienovick

jamienovick commented Mar 31, 2025

Copy link
Copy Markdown

@shahrukh-compuco

I have added the technical details section to the PR

Many thanks... but perhaps it should include some more of your thinking after answering the questions below.

also along with that this issue does not exist and its working fine on core as in latest version of civi the display has quiet changed in terms of code as now civi is using search kit to display memberships on membership tab also regarding the breaking chances its the code that was being used in civicrm core with just a slight modification that we refresh the membership tab as soon as any relationship gets updated

I'm a bit confused by this. When you say: "its working fine on core, as in latest version of civi the display has quiet changed in terms of code as now civi is using search kit to display memberships on membership tab" - I can see that the version of CiviCRM that we use in compuclient 4.x and that this is targeted at (5.75) has relationships using search kit already, so I'm confused by what you are indicating here. If it's working fine in core, why do we need this fix?

also regarding the breaking chances its the code that was being used in civicrm core with just a slight modification that we refresh the membership tab as soon as any relationship gets updated

Are you saying this was a problem in 5.75 which is fixed in newer CiviCRM (i.e. 6.0+) and you are backporting the fix? Or something else?

but still I would mention these cases to test in ticket and for and for future tests I think it would be ok only to test the issue mentioned in ticket

Well ideally we wouldn't need to add additional test cases that we need to perform as this takes more time with each release. Is there a way around this?

@shahrukh-compuco

Copy link
Copy Markdown
Contributor Author

@shahrukh-compuco

I have added the technical details section to the PR

Many thanks... but perhaps it should include some more of your thinking after answering the questions below.

also along with that this issue does not exist and its working fine on core as in latest version of civi the display has quiet changed in terms of code as now civi is using search kit to display memberships on membership tab also regarding the breaking chances its the code that was being used in civicrm core with just a slight modification that we refresh the membership tab as soon as any relationship gets updated

I'm a bit confused by this. When you say: "its working fine on core, as in latest version of civi the display has quiet changed in terms of code as now civi is using search kit to display memberships on membership tab" - I can see that the version of CiviCRM that we use in compuclient 4.x and that this is targeted at (5.75) has relationships using search kit already, so I'm confused by what you are indicating here. If it's working fine in core, why do we need this fix?

also regarding the breaking chances its the code that was being used in civicrm core with just a slight modification that we refresh the membership tab as soon as any relationship gets updated

Are you saying this was a problem in 5.75 which is fixed in newer CiviCRM (i.e. 6.0+) and you are backporting the fix? Or something else?

but still I would mention these cases to test in ticket and for and for future tests I think it would be ok only to test the issue mentioned in ticket

Well ideally we wouldn't need to add additional test cases that we need to perform as this takes more time with each release. Is there a way around this?

@jamienovick yes in the most recent version of civi and in 5.75.0 for relationship tab both are using search kit but for membership the latest civicrm version is using search kit whereas 5.75.0 is not using search kit so I think this issue might get fixed automatically when we update the civicrm so this change is only for the fix in our civi version and for additional test I can see if adding a frontend tests for it will be possible or not for this scenario

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants