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 active list item handling #213

Open
wants to merge 1 commit into
base: main
Choose a base branch
from

Conversation

raviks789
Copy link
Contributor

@raviks789 raviks789 commented Jun 20, 2024

fix #212

requires Icinga/ipl-web#225

@cla-bot cla-bot bot added the cla/signed CLA is signed by all contributors of a PR label Jun 20, 2024
@raviks789 raviks789 self-assigned this Jun 20, 2024
Copy link
Member

@ncosta-ic ncosta-ic left a comment

Choose a reason for hiding this comment

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

We should stay consistent and use Icinga\Module\Notifications\Common\Links for path references to strengthen the maintainability.

library/Notifications/Widget/ItemList/ContactGroupList.php Outdated Show resolved Hide resolved
library/Notifications/Widget/ItemList/ContactList.php Outdated Show resolved Hide resolved
library/Notifications/Widget/ItemList/EventList.php Outdated Show resolved Hide resolved
library/Notifications/Widget/ItemList/EventRuleList.php Outdated Show resolved Hide resolved
library/Notifications/Widget/ItemList/IncidentList.php Outdated Show resolved Hide resolved
library/Notifications/Widget/ItemList/ScheduleList.php Outdated Show resolved Hide resolved
@raviks789 raviks789 force-pushed the fix/active-list-item-handling branch from 3e84c7a to 2653a53 Compare June 21, 2024 08:35
ncosta-ic

This comment was marked as duplicate.

ncosta-ic
ncosta-ic previously approved these changes Jun 21, 2024
$this->getAttributes()->set('data-action-item', true);
$this->getAttributes()
->set('data-action-item', true)
->set('data-icinga-detail-filter', QueryString::render(Filter::equal('id', $this->item->id)));
Copy link
Member

Choose a reason for hiding this comment

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

We should stay consistent and use Icinga\Module\Notifications\Common\Links for path references to strengthen the maintainability.

I'd extend this to query param references:

Links::contactGroup($this->item->id)->getQueryString()

@raviks789 raviks789 force-pushed the fix/active-list-item-handling branch from 2653a53 to 68f7fd1 Compare June 26, 2024 08:15
@raviks789 raviks789 requested a review from nilmerg June 26, 2024 13:14
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
cla/signed CLA is signed by all contributors of a PR
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Fix active list item handling
3 participants