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

Fix #4049 Nested tags not showing under some circumstances #25218

Closed
wants to merge 0 commits into from

Conversation

gellweiler
Copy link
Contributor

This commit fixes #4049.

Bug description: When nested tags were not created in the same order as they are nested, they are not shown in the add tag to contact dialog.

See: https://lab.civicrm.org/dev/core/-/issues/4049

@civicrm-builder
Copy link

Can one of the admins verify this patch?

@civibot
Copy link

civibot bot commented Dec 24, 2022

(Standard links)

@civibot civibot bot added the master label Dec 24, 2022
@eileenmcnaughton eileenmcnaughton changed the title Fix #4049 Fix #4049 Nested tags not showing under some circumstances Dec 27, 2022
@eileenmcnaughton
Copy link
Contributor

jenkins add to whitelist

@gellweiler
Copy link
Contributor Author

gellweiler commented Dec 28, 2022

Fixed the style issue. Perhaps we should also unit test the tag tree algorithm to prevent this or a similar bug from reemerging. What are your thoughts on that?

@eileenmcnaughton
Copy link
Contributor

@gellweiler I'm always in favour of unit tests - looking I can't see any existing tests on that function - in fact the class CRM_Core_BAO_TagTest doesn't even exist! - are you comfortable creating a test?

@gellweiler
Copy link
Contributor Author

Ok I will write a test this week. I have no experience with mocking the db in civicrm but I guess I can just look at other tests as a reference. I tend to write more Java code these days than PHP so I'm not that familiar with the typical test frameworks in PHP but copy and paste I guess will get the me there.

@gellweiler
Copy link
Contributor Author

@eileenmcnaughton I've added a test by ammending my commit and force pushing to the branch. Apparently that closed the PR. Can you open it again? I use mostly gitlab these days and I'm a bit confused of the behavior of GitHub in this instance.

@gellweiler
Copy link
Contributor Author

gellweiler commented Jan 1, 2023

@eileenmcnaughton Ok so what happend is I accidentially removed my commit from my fork and did a force push. Apprently that closed the PR. I had to open a new PR. Sorry about that: #25254

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