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.

Sign up for GitHub

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

Jump to bottom

Fix editing of multi-selected services with same name #2801

Merged
merged 1 commit into from
Sep 21, 2023
Merged

Fix editing of multi-selected services with same name #2801

merged 1 commit into from
Sep 21, 2023

Conversation

raviks789
Copy link
Collaborator

fixes #2798

@raviks789 raviks789 added the bug label Sep 12, 2023
@raviks789 raviks789 self-assigned this Sep 12, 2023
@cla-bot cla-bot bot added the cla/signed label Sep 12, 2023
@moreamazingnick
Copy link
Contributor

moreamazingnick commented Sep 14, 2023 •
edited
Loading

why a : and not a ! similar to the official icinga syntax, like in my merge request from Aug 31, 2022 ?

also : is not forbidden as a servicename or hostname

@Thomas-Gelf Thomas-Gelf added this to the v1.11.0 milestone Sep 21, 2023
@Thomas-Gelf Thomas-Gelf force-pushed the fix/multiselected-service-same-name branch from 83cc96d to 23fee20 Compare September 21, 2023 12:21
@Thomas-Gelf Thomas-Gelf self-requested a review September 21, 2023 12:29
Thomas-Gelf
Thomas-Gelf requested changes Sep 21, 2023
View reviewed changes
Copy link
Contributor

@Thomas-Gelf Thomas-Gelf left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Key is used for visualization purposes only, so we could use ': ' (trailing space) as a separator. If you check for instanceof IcingaService instead of $type === 'service', phpstan should no longer complain

@raviks789 raviks789 force-pushed the fix/multiselected-service-same-name branch 3 times, most recently from bb71249 to 4c8670c Compare September 21, 2023 12:43
@raviks789
Fix editing of multi-selected services with same name
7c9cd6a 
Since the object names function as keys for the multi-selected objects array, this becomes a problem when
multiple services as the same name. Hence for the services the array keys must be a combination of service name
and the host name to which the service is related.
@raviks789 raviks789 force-pushed the fix/multiselected-service-same-name branch from 4c8670c to 7c9cd6a Compare September 21, 2023 12:51
@Thomas-Gelf Thomas-Gelf merged commit 63a3e6a into master Sep 21, 2023
12 checks passed
@Thomas-Gelf Thomas-Gelf deleted the fix/multiselected-service-same-name branch September 21, 2023 12:59
@Thomas-Gelf
Copy link
Contributor

Merged, thank you!

@Thomas-Gelf Thomas-Gelf mentioned this pull request Sep 21, 2023
Closed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@Thomas-Gelf Thomas-Gelf Thomas-Gelf requested changes

Assignees

@raviks789 raviks789

Labels
bug cla/signed
Projects
None yet
Milestone
v1.11.0
Development

Successfully merging this pull request may close these issues.

Can't edit multiple services with same name
3 participants
@raviks789 @moreamazingnick @Thomas-Gelf