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

Avoid null column showing on groups search page #22724

Merged

Conversation

braders
Copy link
Contributor

@braders braders commented Feb 7, 2022

Overview

Avoid null column showing on groups search page.

Before

Currently, if you setup a child group (e.g. a mailing group) and then try to view that group, an extra column is shown with the text content "null". For example, as seen on dmaster:

Screenshot 2022-02-07 at 20 20 28

After

The extra "null" column does not appear:

Screenshot 2022-02-07 at 20 24 56

Technical Details

The group search page had some JavaScript checking whether "0" or "1" were truthy. As both were strings both were truthy. Therefore, now a proper boolean is used which works correctly.

Comments

I belive the extra column is intended to show the organisation in a multisite setup. I don't have such a site to hand to test with, so whilst I'm reasonably confident it might be nice for someone to sense-check it all looks good from that angle.

I've also removed the empty() check whilst I'm here as it's problamatic with the new escaping mode. $showOrgInfo appears to always be set to either true or false.

@civibot
Copy link

civibot bot commented Feb 7, 2022

(Standard links)

@civibot civibot bot added the master label Feb 7, 2022
@demeritcowboy demeritcowboy added the merge ready PR will be merged after a few days if there are no objections label Feb 15, 2022
@demeritcowboy
Copy link
Contributor

I'm not sure about the multisite but this works. Cross-ref'ing to https://lab.civicrm.org/dev/core/-/issues/3071

@demeritcowboy demeritcowboy merged commit 05220f2 into civicrm:master Feb 17, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
master merge ready PR will be merged after a few days if there are no objections
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants