Conversation
|
@copilot can you create a new PR based on this one to update the test cases matching the code changes? |
Documentation build overview
Show files changed (2 files in total): 📝 2 modified | ➕ 0 added | ➖ 0 deleted
|
| return diff.seconds | ||
|
|
||
|
|
||
| class VersionAutomationRule(PolymorphicModel, TimeStampedModel): |
There was a problem hiding this comment.
I will delete this model in a following PR and migration. We are not using it anymore.
stsewd
left a comment
There was a problem hiding this comment.
Looks like this is missing moving the models and forms to the projects app.
| rule_triggered_for_project = True | ||
| rule_triggered_for_version = True | ||
| rule.run(version) | ||
|
|
||
| # We only trigger the first matching rule, | ||
| # We only trigger the first matching rule per version | ||
| # to avoid triggering multiple builds for the same tag/branches. | ||
| break | ||
| if rule_triggered_for_version: | ||
| break |
There was a problem hiding this comment.
There is no need to track this in two variables. Since you only want to know if at least one rule matched, you can use a single variable.
| rule_triggered_for_project = True | |
| rule_triggered_for_version = True | |
| rule.run(version) | |
| # We only trigger the first matching rule, | |
| # We only trigger the first matching rule per version | |
| # to avoid triggering multiple builds for the same tag/branches. | |
| break | |
| if rule_triggered_for_version: | |
| break | |
| rule_triggered = True | |
| rule.run(version) | |
| # We only trigger the first matching rule per version | |
| # to avoid triggering multiple builds for the same tag/branches. | |
| break |
There was a problem hiding this comment.
We need to check all the rules for all the versions. Using only one variable, if the first rule is triggered on the first version, we will be skipping the other versions.
This reverts commit 19b5ada.
agjohnson
left a comment
There was a problem hiding this comment.
There's some needed on the UI side but overall this looks good.
| project = forms.CharField(widget=forms.HiddenInput(), required=False) | ||
|
|
||
| VERSION_TYPE_CHOICES = [ | ||
| RichChoice(text=name, value=value, description="description", disabled=False) |
There was a problem hiding this comment.
Note to update the description here. The description is a good place to describe the types in more detail, instead of cramming that information into the option name.
There was a problem hiding this comment.
I added description because it was mandatory, basically. I don't know what could be a good description here. They are just version types 🤷🏼 . Do you have any suggestion for each of them?
There was a problem hiding this comment.
Description is the main reason to use RichSelect. Otherwise if you just need a basic list of items you would just continue using a basic select widget. RichSelect is only needed if you want additional information like a description or image with each option.
In this case, there is a Django multi select widget that you probably want to use. I`m pretty sure we've used it elsewhere, I think with team project selection -- if you need an example.
There was a problem hiding this comment.
Yeah, I only want a multi select field, but I wasn't able to find an example where we use it. I want exactly what I posted in the video (#12848 (comment)) but without the description.
| class CommaSeparatedMultipleChoiceField(forms.MultipleChoiceField): | ||
| """Handle comma-separated values for a single hidden input.""" | ||
|
|
||
| hidden_widget = forms.HiddenInput | ||
|
|
||
| def to_python(self, value): | ||
| if isinstance(value, str): | ||
| value = [item.strip() for item in value.split(",") if item.strip()] | ||
| return super().to_python(value) | ||
|
|
||
| def prepare_value(self, value): | ||
| if isinstance(value, (list, tuple)): | ||
| return ",".join(value) | ||
| return super().prepare_value(value) |
There was a problem hiding this comment.
Why is this a comma separated value? It's not really clear what UX you're aiming for here, but I think you likely want something that SUI already gives us.
There was a problem hiding this comment.
I had to use a comma separated value because of SUI actually 😄 . That's how it handle multiple choice fields. It ends up with value=branch,tag
The UX is to be able to select multiple items. It works as I want now:
Peek.2026-04-09.10-59.webm
There was a problem hiding this comment.
Yeah that looks great. There are other options for getting that data into the SUI module without altering the data on the application side. But the effect is identical either way.

This is the initial proposal to migrate our current
VersionAutomationRulesandRegexAutomationRulesto only one model calledAutomationRuleas discussed.I created a basic UI just to be able to use the feature; but we need to work on ext-theme for that. I think it won't be a lot of work, but we need:
version_typesshould be a multi-select fieldversion_match_patternshould be shown/hidden based on another input valuev2#12788