Skip to content

Avoids unnecessary additional domain save notification publishing when sorting an already sorted collection of domains #19106

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

Conversation

AndyButland
Copy link
Contributor

Prerequisites

  • I have added steps to test this contribution in the description below

Resolves: #19104

Description

The behaviour described in the linked issue occurs due to two operations being requested on the domains collection:

  • A save for each domain
  • A sort for each domain that's not already sorted

In most cases, we aren't sorting as well as saving, so we can short-circuit and not publish the second notification when we aren't making any changes to sort order.

When we are sorting, it's strictly correct to publish two notifications as we are making two separate saves.

…en sorting an already sorted collection of domains.
@Copilot Copilot AI review requested due to automatic review settings April 22, 2025 13:47
@AndyButland AndyButland changed the title Avoids unnecessary additional domain save notification publishing when sorting an already sorted collection of domains. Avoids unnecessary additional domain save notification publishing when sorting an already sorted collection of domains Apr 22, 2025
Copy link
Contributor

@Copilot Copilot AI left a comment

Choose a reason for hiding this comment

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

Pull Request Overview

This PR aims to prevent unnecessary notifications when the domains collection is already sorted, avoiding duplicate domain save notifications when no sorting is required.

  • Conditionally short-circuiting notification publishing if the domains array is empty or already sorted
  • Introducing the AreDomainsAlreadySorted helper method to assess the sort order of domains

Copy link
Contributor

@nikolajlauridsen nikolajlauridsen left a comment

Choose a reason for hiding this comment

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

Looks good, tests good 👍

@nikolajlauridsen nikolajlauridsen merged commit 0fc5b2a into v13/dev May 14, 2025
19 checks passed
@nikolajlauridsen nikolajlauridsen deleted the v13/bugfix/avoid-unnecessary-domain-save-notifications branch May 14, 2025 08:25
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants