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

dev/core#1046 - allow for the real "name" to be specified in xml #15182

Merged

Conversation

demeritcowboy
Copy link
Contributor

Overview

Towards https://lab.civicrm.org/dev/core/issues/1046 (fixing name and label in case roles), methinks this is the smallest change I can make without breaking something. It doesn't fix anything yet, just allows for an override tag in the xml to specify the actual "name", as floated at NY sprint.

Before

After

Technical Details

This is a NFC as long as you don't try to use the new tag yet. At the moment if you try to use this tag in the xml (e.g. in xml files) and then try to change the relationship type label, something will still break somewhere.

Comments

Next step is to write out the tag to the xml when the case type gets updated in the UI. At the moment it would still need to have the same value as label.

@civibot
Copy link

civibot bot commented Sep 1, 2019

(Standard links)

@civibot civibot bot added the master label Sep 1, 2019
@demeritcowboy
Copy link
Contributor Author

test this please - should be ok now since the failure is obviously a previously noted weirdness around Feb 29 in leap years where the test calculates "+1 year" as march 1st but the code calculates it as Feb 28th (and both are kinda not wrong).

@eileenmcnaughton
Copy link
Contributor

Despite the shortness of the patch my brain struggled to process it. However, once I stepped through the test it made sense - merging. I agree this won't change anything until further work is done.

@eileenmcnaughton eileenmcnaughton merged commit 2d283a2 into civicrm:master Sep 2, 2019
@demeritcowboy demeritcowboy deleted the locate-name-or-label branch September 2, 2019 14:54
@demeritcowboy
Copy link
Contributor Author

Thanks @eileenmcnaughton. You're right looking at it a day later it's a bit hard to follow for me too. I've put up #15192 which might be better.

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.

2 participants