(Smart Group) is being constantly added while editing the smart group title from 'Manage Group' page #20898
Conversation
|
(Standard links)
|
templates/CRM/Group/Form/Search.tpl
Outdated
| @@ -164,6 +165,10 @@ | |||
| .prepend('{/literal}<span class="collapsed show-children" title="{ts}show child groups{/ts}"/></span>{literal}') | |||
| .find('div').css({'display': 'inline'}); | |||
| }); | |||
| $('tbody tr.crm-smart-group', settings.nTable).each(function () { | |||
| $(this).find('td:first') | |||
| .append(smartGroupText); | |||
There was a problem hiding this comment.
One thing is that the parentsOnly var is only set on first page load, not when the table is refreshed by ajax, so changing any of the search filters at the top of the table like Group Type or Visibility then refreshes the table, but doesn't apply this text.
There was a problem hiding this comment.
Hi @demeritcowboy thanks for your feedback. I have updated my patch and also checked with parent group. Can you please review my patch again?
There was a problem hiding this comment.
Thanks it's better, except now it adds "Smart Group" to every child group even if they are not smart kids. Hmm this seems harder than it looks.
|
I have tested the latest patch, and it seems to work well:
|
|
In my testing even dumb child groups have the smart label added - did that come up for you? |
|
@demeritcowboy oh, yes, you're right. Child groups, even if static, still get the label. |
templates/CRM/Group/Form/Search.tpl
Outdated
| } | ||
| $('tbody tr.crm-smart-group', settings.nTable).each(function () { | ||
| $(this).find('td:first') | ||
| .append(smartGroupText); |
There was a problem hiding this comment.
Could this be simplified to:
$('tbody tr.crm-smart-group > td.crm-group-name', settings.nTable).append(smartGroupText);
(I did a bit of testing, seems to work)
There was a problem hiding this comment.
Ah right, yep it works for me too. Will update the PR.
templates/CRM/Group/Form/Search.tpl
Outdated
| @@ -234,15 +239,18 @@ | |||
| 'parent_is_' + parent_id, | |||
| 'crm-row-child' | |||
| ]; | |||
| if (!$.inArray("crm-smart-group", val.row_classes)) { | |||
| smartGroupText = ''; | |||
| } | |||
There was a problem hiding this comment.
Try this instead:
if ('DT_RowClass' in val) {
val.row_classes = val.row_classes.concat(val.DT_RowClass.split(' ').filter((item) => val.row_classes.indexOf(item) < 0));
// this ->
if (val.DT_RowClass.indexOf('crm-smart-group') == -1) {
smartGroupText = '';
}
}
because the val.row_classes, set just above, will not have the crm-smart-group class. It gets set from the DT_RowClass in the condition right under.
There was a problem hiding this comment.
Awesome, thanks for the patch :)
|
Thank you @mlufty and @demeritcowboy for reviewing the patch and mentioning the different use-cases that lead to errors. And I found another issue where if you search for a child group, and then try to edit the child group title, it adds 'childGroup' subtext in it. It seems like a separate issue but will create a separate PR. The intention is to prevent the indicator text to appear on the title field on the inline edit. |
|
@demeritcowboy This PR looks good to me now. We've tested it in production and haven't had any complaints. Any thoughts? |
|
Oh did the PR get updated? It looks like it still has the original code? |
|
My bad, I need more coffee! @monishdeb Could you update? |
… title from 'Manage Group' page
|
@mlutfy @demeritcowboy my apologies for the delay. I have updated the PR and looks ok to me: Also checked with Level > 2 parent group, which contains smart group as child group. |
|
jenkins retest this please - strange error: |
|
As noted there's some issues when using the filters but they're also wonky before. Looks good. |
Overview
Steps to replicate:
Before
After