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

Prevent scheduling a new rule with a duplicate alias #3029

Conversation

michellemounde
Copy link
Contributor

Fixes #2942

  1. Add duplicate alias check at API level to RuleScheduledChangesView

Before insert:
Screenshot from 2023-10-23 14-45-45

On insert:
Screenshot from 2023-10-23 14-45-54

Copy link
Contributor

@bhearsum bhearsum left a comment

Choose a reason for hiding this comment

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

This is an OK start, but more is needed. Specifically, we should reject duplicate aliases when:

  • The alias exists in any row in the rules table
  • The alias exists in any row in the rule_scheduled_changes table that meets both the following conditions:
    • complete is False
    • change_type is insert or update

We also need tests for these cases in https://github.com/mozilla-releng/balrog/blob/main/tests/admin/views/test_rules.py

@michellemounde
Copy link
Contributor Author

Ok @bhearsum will work on the changes. I have a question regarding the failing check. I've looked at the details but I am not sure how to fix the issue so that the check passes.

@bhearsum
Copy link
Contributor

Ok @bhearsum will work on the changes. I have a question regarding the failing check. I've looked at the details but I am not sure how to fix the issue so that the check passes.

That error looks unrelated to your change. It will probably fix itself the next time you push - don't worry about it.

@bhearsum
Copy link
Contributor

This ended up getting fixed in #3010 first.

@bhearsum bhearsum closed this Oct 27, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Balrog accepts scheduled rule changes with duplicate alias'
2 participants