diff --git a/reviewboard/reviews/tests/test_conditions.py b/reviewboard/reviews/tests/test_conditions.py index 78c60bb1c92370e9d69e8ef170e396b6a5cdc44b..318abbfa43857aec40e705230ac59b2b47e82e47 100644 --- a/reviewboard/reviews/tests/test_conditions.py +++ b/reviewboard/reviews/tests/test_conditions.py @@ -754,7 +754,7 @@ class ReviewRequestRepositoryTypeChoiceTests(TestCase): condition_set = ConditionSet(ConditionSet.MODE_ALL, [ Condition(self.choice, self.choice.get_operator('one-of'), - [repository1.tool, repository2.tool]) + [repository1.scmtool_id, repository2.scmtool_id]) ]) self.assertTrue(condition_set.matches( @@ -777,7 +777,7 @@ class ReviewRequestRepositoryTypeChoiceTests(TestCase): condition_set = ConditionSet(ConditionSet.MODE_ALL, [ Condition(self.choice, self.choice.get_operator('not-one-of'), - [repository1.tool, repository2.tool]) + [repository1.scmtool_id, repository2.scmtool_id]) ]) self.assertFalse(condition_set.matches( diff --git a/reviewboard/scmtools/conditions.py b/reviewboard/scmtools/conditions.py index f03484ba9b269813c3a426bb070175f43145e7ba..be585b2ef4b020840789a97eaf3cef64469b2a18 100644 --- a/reviewboard/scmtools/conditions.py +++ b/reviewboard/scmtools/conditions.py @@ -2,6 +2,7 @@ from django.utils.translation import gettext_lazy as _ from djblets.conditions.choices import (BaseConditionModelMultipleChoice, + BaseConditionChoice, ConditionChoices) from djblets.conditions.operators import (AnyOperator, BaseConditionOperator, @@ -9,7 +10,9 @@ from djblets.conditions.operators import (AnyOperator, IsNotOneOfOperator, IsOneOfOperator, UnsetOperator) +from djblets.conditions.values import ConditionValueMultipleChoiceField +from reviewboard.scmtools import scmtools_registry from reviewboard.scmtools.models import Repository, Tool from reviewboard.site.models import LocalSite @@ -137,7 +140,7 @@ class RepositoriesChoice(RepositoryConditionChoiceMixin, class RepositoryTypeChoice(RepositoryConditionChoiceMixin, - BaseConditionModelMultipleChoice): + BaseConditionChoice): """A condition choice for matching repository types. This is used to match a :py:class:`~reviewboard.scmtools.models.Repository` @@ -145,7 +148,6 @@ class RepositoryTypeChoice(RepositoryConditionChoiceMixin, :py:class:`~reviewboard.scmtools.models.Tool`). """ - queryset = Tool.objects.all() choice_id = 'repository_type' name = _('Repository type') @@ -154,11 +156,33 @@ class RepositoryTypeChoice(RepositoryConditionChoiceMixin, IsNotOneOfOperator, ]) + def default_value_field(self, **kwargs): + """Return the default value field for this choice. + + This will call out to :py:meth:`get_queryset` before returning the + field, allowing subclasses to simply set :py:attr:`queryset` or to + perform more dynamic queries before constructing the form field. + + Args: + **kwargs (dict): + Extra keyword arguments for this function, for future + expansion. + + Returns: + djblets.conditions.values.ConditionValueMultipleModelField: + The form field for the value. + """ + repository_type_choices = [ + (scmtool.scmtool_id, scmtool.name) + for scmtool in scmtools_registry + ] + return ConditionValueMultipleChoiceField( + choices=repository_type_choices) + def get_match_value(self, repository, **kwargs): """Return the value used for matching. - This will return the :py:class:`~reviewboard.scmtools.models.Tool` - for the provided repository. + This will return the SCMTool ID for the provided repository. Args: repository (reviewboard.scmtools.models.Repository): @@ -168,11 +192,11 @@ class RepositoryTypeChoice(RepositoryConditionChoiceMixin, Unused keyword arguments. Returns: - reviewboard.scmtools.models.Tool: - The repository's tool. + str: + The SCMTool ID for the repository's tool. """ if repository: - return repository.tool + return repository.scmtool_id else: return None diff --git a/reviewboard/scmtools/management/commands/registerscmtools.py b/reviewboard/scmtools/management/commands/registerscmtools.py deleted file mode 100644 index 0cf366ee9e1bdf1adb5c44e4f0a2d87a129f563b..0000000000000000000000000000000000000000 --- a/reviewboard/scmtools/management/commands/registerscmtools.py +++ /dev/null @@ -1,39 +0,0 @@ -"""Management command to register SCMTools in the database.""" - -from django.core.management.base import BaseCommand -from django.utils.translation import gettext as _, ngettext - -from reviewboard.scmtools.models import Tool - - -class Command(BaseCommand): - """Management command to register SCMTools in the database.""" - - help = _('Register available SCMTools in the database.') - - def handle(self, **options): - """Handle the command. - - Args: - **options (dict, unused): - Options parsed on the command line. For this command, no - options are available. - """ - new_tools = Tool.objects.register_from_entrypoints() - - if new_tools: - count = len(new_tools) - - self.stdout.write( - ngettext('Registered %(count)d new SCMTool: %(tools)s\n', - 'Registered %(count)d new SCMTools: %(tools)s\n', - count) - % { - 'count': count, - 'tools': ', '.join( - tool.name - for tool in new_tools - ) - }) - else: - self.stdout.write(_('No new SCMTools were found.\n')) diff --git a/reviewboard/scmtools/tests/test_conditions.py b/reviewboard/scmtools/tests/test_conditions.py index e334d692995ff336def4902fd43049beb7721b21..dd6716ebdb213c81f74372048d363a8c969bb80b 100644 --- a/reviewboard/scmtools/tests/test_conditions.py +++ b/reviewboard/scmtools/tests/test_conditions.py @@ -283,7 +283,7 @@ class RepositoryTypeChoiceTests(TestCase): condition_set = ConditionSet(ConditionSet.MODE_ALL, [ Condition(self.choice, self.choice.get_operator('one-of'), - [repository1.tool, repository2.tool]) + [repository1.scmtool_id, repository2.scmtool_id]), ]) self.assertTrue(condition_set.matches(repository=repository1)) @@ -301,7 +301,7 @@ class RepositoryTypeChoiceTests(TestCase): condition_set = ConditionSet(ConditionSet.MODE_ALL, [ Condition(self.choice, self.choice.get_operator('not-one-of'), - [repository1.tool, repository2.tool]) + [repository1.scmtool_id, repository2.scmtool_id]), ]) self.assertFalse(condition_set.matches(repository=repository1)) diff --git a/reviewboard/upgrade.py b/reviewboard/upgrade.py index b8048442f38aeb5edfc17a2c0b2d5fa7352b4831..61a016ad936f6f16bc195104fe0b7bd933c3f729 100644 --- a/reviewboard/upgrade.py +++ b/reviewboard/upgrade.py @@ -173,6 +173,78 @@ def post_upgrade_apply_scmtool_data(upgrade_state): repositories.update(scmtool_id=scmtool_id) +def pre_upgrade_store_condition_tool_info(upgrade_state): + """Store the data for converting RepositoryTypeChoice data. + + The :py:class:`reviewboard.scmtools.conditions.RepositoryTypeChoice` + traditionally used the Tool pk as its value. This upgrade sequence will + convert those to use the SCMTool ID instead. + + This must run after :py:func:`pre_upgrade_store_scmtool_data`. + + Version Added: + 5.0 + + Args: + upgrade_state (dict): + Upgrade state that can be used by pre-upgrade/post-upgrade steps. + """ + if upgrade_state['needs_scmtool_id_migration']: + from reviewboard.integrations.models import IntegrationConfig + from reviewboard.scmtools.models import Tool + + tool_pks = set() + affected_configs = set() + + for config in IntegrationConfig.objects.all(): + for condition in config.get('conditions')['conditions']: + if condition['choice'] == 'repository_type': + tool_pks.update(condition['value']) + affected_configs.add(config.pk) + + tools = Tool.objects.filter(pk__in=tool_pks).only('pk', 'class_name') + + upgrade_state['tool_pk_to_scmtool_id'] = { + tool.pk: tool.scmtool_id + for tool in tools + } + upgrade_state['conditions_for_scmtool_migration'] = affected_configs + + +def post_upgrade_apply_condition_tool_info(upgrade_state): + """Convert RepositoryTypeChoice conditions to use SCMTool ID. + + The :py:class:`reviewboard.scmtools.conditions.RepositoryTypeChoice` + traditionally used the Tool pk as its value. This upgrade sequence will + convert those to use the SCMTool ID instead. + + Version Added: + 5.0 + + Args: + upgrade_state (dict): + Upgrade state that can be used by pre-upgrade/post-upgrade steps. + """ + if upgrade_state['needs_scmtool_id_migration']: + from reviewboard.integrations.models import IntegrationConfig + + tool_pk_to_scmtool_id = upgrade_state['tool_pk_to_scmtool_id'] + config_pks = upgrade_state['conditions_for_scmtool_migration'] + + for config in IntegrationConfig.objects.filter(pk__in=config_pks): + conditions = config.get('conditions') + + for condition in conditions['conditions']: + if condition['choice'] == 'repository_type': + condition['value'] = [ + tool_pk_to_scmtool_id[pk] + for pk in condition['value'] + ] + + config.set('conditions', conditions) + config.save(update_fields=('settings',)) + + def run_pre_upgrade_tasks(upgrade_state): """Run any database pre-upgrade tasks. @@ -187,6 +259,7 @@ def run_pre_upgrade_tasks(upgrade_state): """ pre_upgrade_reset_oauth2_provider(upgrade_state) pre_upgrade_store_scmtool_data(upgrade_state) + pre_upgrade_store_condition_tool_info(upgrade_state) def run_post_upgrade_tasks(upgrade_state): @@ -201,3 +274,4 @@ def run_post_upgrade_tasks(upgrade_state): """ post_upgrade_reset_oauth2_provider(upgrade_state) post_upgrade_apply_scmtool_data(upgrade_state) + post_upgrade_apply_condition_tool_info(upgrade_state)