diff --git a/reviewboard/scmtools/forms.py b/reviewboard/scmtools/forms.py index 564b63a6f941941de5eda6788030a2f206fa7592..f556bfe65dd8f3dc2782f9c98ae92a0eb9b7d429 100644 --- a/reviewboard/scmtools/forms.py +++ b/reviewboard/scmtools/forms.py @@ -1,6 +1,7 @@ import logging import sys from itertools import chain +from typing import Any, Dict, List, Type from django import forms from django.contrib.admin.widgets import FilteredSelectMultiple @@ -26,10 +27,12 @@ from reviewboard.hostingsvcs.errors import (AuthorizationError, TwoFactorAuthCodeRequiredError) from reviewboard.hostingsvcs.fake import FAKE_HOSTING_SERVICES from reviewboard.hostingsvcs.models import HostingServiceAccount -from reviewboard.hostingsvcs.service import (get_hosting_services, +from reviewboard.hostingsvcs.service import (HostingService, + get_hosting_services, get_hosting_service) from reviewboard.reviews.models import Group from reviewboard.scmtools import scmtools_registry +from reviewboard.scmtools.core import SCMTool from reviewboard.scmtools.errors import (AuthenticationError, RepositoryNotFoundError, SCMError, @@ -932,7 +935,7 @@ class RepositoryForm(LocalSiteAwareModelFormMixin, forms.ModelForm): for class_name, cls in FAKE_HOSTING_SERVICES.items(): if class_name not in hosting_services: service_info = self._get_hosting_service_info(cls) - service_info['fake'] = True + service_info['isFake'] = True self.hosting_service_info[cls.hosting_service_id] = \ service_info @@ -985,7 +988,7 @@ class RepositoryForm(LocalSiteAwareModelFormMixin, forms.ModelForm): scmtool_choices.append((scmtool_id, name)) self.scmtool_info[scmtool_id] = { 'name': name, - 'fake': True, + 'isFake': True, } scmtool_choices.sort(key=lambda x: x[1]) @@ -1066,6 +1069,27 @@ class RepositoryForm(LocalSiteAwareModelFormMixin, forms.ModelForm): return self.local_site.name + def get_js_model_data( + self, + ) -> Dict[str, Any]: + """Return data for the RepositoryForm model on the page. + + Version Added: + 6.0 + + Returns: + dict: + The form model data. + """ + return { + 'options': { + 'hostingServices': sorted(self.hosting_service_info.values(), + key=lambda info: info['id']), + 'tools': sorted(self.scmtool_info.values(), + key=lambda info: info['id']), + }, + } + def iter_subforms(self, bound_only=False, with_auth_forms=False): """Iterate through all subforms matching the given criteria. @@ -1138,7 +1162,10 @@ class RepositoryForm(LocalSiteAwareModelFormMixin, forms.ModelForm): Repository.PATH_CONFLICT_ERROR in self.errors.get('path', []) ) - def _get_scmtool_info(self, scmtool_cls): + def _get_scmtool_info( + self, + scmtool_cls: Type[SCMTool], + ) -> Dict[str, Any]: """Return the information for a SCMTool. Args: @@ -1150,17 +1177,20 @@ class RepositoryForm(LocalSiteAwareModelFormMixin, forms.ModelForm): dict: Information about the SCMTool. """ - info = {} - - for attr in ('name', - 'supports_pending_changesets', - 'supports_post_commit'): - info[attr] = getattr(scmtool_cls, attr) - - return info + return { + 'id': scmtool_cls.scmtool_id, + 'name': scmtool_cls.name, + 'supportsPendingChangesets': + scmtool_cls.supports_pending_changesets, + 'suportsPostCommit': scmtool_cls.supports_post_commit, + } - def _get_hosting_service_info(self, hosting_service, hosting_accounts=[], - is_instance_service=False): + def _get_hosting_service_info( + self, + hosting_service: Type[HostingService], + hosting_accounts: List[HostingServiceAccount] = [], + is_instance_service: bool = False, + ) -> Dict[str, Any]: """Return the information for a hosting service. Args: @@ -1174,7 +1204,7 @@ class RepositoryForm(LocalSiteAwareModelFormMixin, forms.ModelForm): :py:class:`~reviewboard.hostingsvcs.models. HostingServiceAccount`s. - is_active (boolean, optional): + is_active (bool, optional): Whether this hosting service is currently active, based on an existing repository being configured. @@ -1201,23 +1231,24 @@ class RepositoryForm(LocalSiteAwareModelFormMixin, forms.ModelForm): scmtools = visible_scmtools return { + 'id': hosting_service.hosting_service_id, + 'name': hosting_service.name, 'scmtools': sorted(scmtools), 'plans': [], - 'planInfo': {}, - 'self_hosted': hosting_service.self_hosted, - 'needs_authorization': hosting_service.needs_authorization, - 'supports_bug_trackers': hosting_service.supports_bug_trackers, - 'supports_ssh_key_association': + 'isSelfHosted': hosting_service.self_hosted, + 'needsAuthorization': hosting_service.needs_authorization, + 'supportsBugTrackers': hosting_service.supports_bug_trackers, + 'supportsSSHKeyAssociation': hosting_service.supports_ssh_key_association, - 'supports_two_factor_auth': + 'supportsTwoFactorAuth': hosting_service.supports_two_factor_auth, - 'needs_two_factor_auth_code': False, + 'needsTwoFactorAuthCode': False, 'accounts': [ { - 'pk': account.pk, - 'hosting_url': account.hosting_url, + 'id': account.pk, + 'hostingURL': account.hosting_url, + 'isAuthorized': account.is_authorized, 'username': account.username, - 'is_authorized': account.is_authorized, } for account in hosting_accounts if account.service_name == hosting_service.hosting_service_id @@ -1322,7 +1353,10 @@ class RepositoryForm(LocalSiteAwareModelFormMixin, forms.ModelForm): :py:class:`~reviewboard.hostingsvcs.forms.HostingServiceForm`. """ repository = self.instance - plan_info = {} + plan_info = { + 'id': plan_type_id, + 'name': str(plan_type_label), + } if hosting_service.supports_repositories: # We only want to load repository data into the form if it's meant @@ -1373,12 +1407,8 @@ class RepositoryForm(LocalSiteAwareModelFormMixin, forms.ModelForm): if self.instance: form.load(repository) - hosting_info = self.hosting_service_info[hosting_service_id] - hosting_info['planInfo'][plan_type_id] = plan_info - hosting_info['plans'].append({ - 'type': plan_type_id, - 'label': str(plan_type_label), - }) + self.hosting_service_info[hosting_service_id]['plans'].append( + plan_info) def _populate_hosting_service_fields(self): """Populates all the main hosting service fields in the form. @@ -1554,7 +1584,7 @@ class RepositoryForm(LocalSiteAwareModelFormMixin, forms.ModelForm): self.errors['hosting_account'] = \ self.error_class([str(e)]) hosting_info = self.hosting_service_info[hosting_type] - hosting_info['needs_two_factor_auth_code'] = True + hosting_info['needsTwoFactorAuthCode'] = True return except AuthorizationError as e: self.errors['hosting_account'] = self.error_class([ diff --git a/reviewboard/scmtools/tests/test_repository_form.py b/reviewboard/scmtools/tests/test_repository_form.py index a132829815f2e6065333adc917217ad0261fec53..9a72e8775b16c205524d31f35d92fecf6dd0fe48 100644 --- a/reviewboard/scmtools/tests/test_repository_form.py +++ b/reviewboard/scmtools/tests/test_repository_form.py @@ -1362,7 +1362,7 @@ class RepositoryFormTests(SpyAgency, TestCase): self.assertEqual(form.errors['hosting_account'], ['Enter your 2FA code.']) self.assertTrue( - form.hosting_service_info['test']['needs_two_factor_auth_code']) + form.hosting_service_info['test']['needsTwoFactorAuthCode']) self.assertEqual( list(form.iter_subforms(bound_only=True)), [ @@ -1391,7 +1391,7 @@ class RepositoryFormTests(SpyAgency, TestCase): self.assertTrue(form.is_valid()) self.assertTrue(form.hosting_account_linked) self.assertFalse( - form.hosting_service_info['test']['needs_two_factor_auth_code']) + form.hosting_service_info['test']['needsTwoFactorAuthCode']) self.assertEqual( list(form.iter_subforms(bound_only=True)), [ diff --git a/reviewboard/static/rb/js/admin/models/repositoryFormModel.es6.js b/reviewboard/static/rb/js/admin/models/repositoryFormModel.es6.js new file mode 100644 index 0000000000000000000000000000000000000000..461fa86953197cb096dea45ad850a684872d581e --- /dev/null +++ b/reviewboard/static/rb/js/admin/models/repositoryFormModel.es6.js @@ -0,0 +1,303 @@ +/** + * Information on an available hosting service account. + * + * This stores basic information on an account on a hosting service, including + * the URL where the service resides, whether the user is currently authorized, + * and the username. + * + * Version Added: + * 6.0 + * + * Model Attributes: + * hostingURL (string): + * The URL of the hosting service, if the associated hosting service + * is a self-hosted service. + * + * isAuthorized (boolean): + * Whether the account is currently authorized (or at least has + * last-known-working authorization credentials set). + * + * username (string): + * The username of the account. + */ +const HostingServiceAccount = Backbone.Model.extend({ + defaults: { + hostingURL: null, + isAuthorized: false, + username: null, + }, +}); + + +/** + * Information on an available hosting service plan. + * + * This is used to differentiate sets of configurations for a hosting service, + * based on some organizational breakdown (such as a user-owned vs. + * organization-owned account). + * + * Version Added: + * 6.0 + * + * Model Attributes: + * bugTrackerRequiresUsername (boolean): + * Whether a bug tracker on this plan requires a username to be + * provided. + * + * name (string): + * The display name of this plan. + */ +const HostingServicePlan = Backbone.Model.extend({ + defaults: { + bugTrackerRequiresUsername: false, + name: null, + }, +}); + + +/** + * Information on a configurable hosting service. + * + * This represents a hosting service that can be selected, if not configuring + * a custom repository. + * + * Version Added: + * 6.0 + * + * Model Attributes: + * isFake (boolean): + * Whether this is a fake (non-operational, information-only) service + * entry. + * + * isSelfHosted (boolean): + * Whether this service is self-hosted (requires an explicit URL during + * account setup). + * + * name (string): + * The display name of the hosting service. + * + * needsAuthorization (boolean): + * Whether authorization is required in order to communicate with this + * service. + * + * needsTwoFactorAuthCode (boolean): + * Whether a two-factor authentication code is required to finish + * configuration. + * + * scmtools (list of string): + * A list of SCMTool IDs or names used by this service. + * + * IDs are preferred, but names exist for backwards-compatibility. + * For most operations, this list should be considered largely opaque. + * + * supportsBugTrackers (boolean): + * Whether this service provides bug tracker support. + * + * supportsSSHKeyAssociation (boolean): + * Whether this service supports associating a configured SSH key. + * + * supportsTwoFactorAuth (boolean): + * Whether this service supports two-factor authentication when + * authorizing an account. + * + * Attributes: + * accounts (Backbone.Collection of RepositoryHostingServiceAccount): + * The collection of accounts available for this service. + * + * plan (Backbone.Collection of RepositoryHostingServicePlan): + * The collection of plans available for this service. + */ +const HostingServiceOption = Backbone.Model.extend({ + defaults: { + isFake: false, + isSelfHosted: false, + name: null, + needsAuthorization: false, + needsTwoFactorAuthCode: false, + scmtools: [], + supportsBugTrackers: false, + supportsSSHKeyAssociation: false, + supportsTwoFactorAuth: false, + }, + + /** + * Parse attributes to set in the service. + * + * This will pull out the ``accounts`` and ``plans`` attributes into + * new :js:attr:`accounts` and :js:attr:`plans` instance-level attributes + * (respectively), and set the rest as Backbone model attributes. + * + * Args: + * data (object): + * The data to parse. + * + * Returns: + * object: + * The parsed attributes to set for the model. + */ + parse(data) { + const newData = { + id: data.id, + isFake: data.isFake, + isSelfHosted: data.isSelfHosted, + name: data.name, + needsAuthorization: data.needsAuthorization, + needsTwoFactorAuthCode: data.needsTwoFactorAuthCode, + scmtools: data.scmtools, + supportsBugTrackers: data.supportsBugTrackers, + supportsSSHKeyAssociation: data.supportsSSHKeyAssociation, + supportsTwoFactorAuth: data.supportsTwoFactorAuth, + }; + + this.accounts = new Backbone.Collection( + data.accounts, + { + model: HostingServiceAccount, + parse: true, + }); + + this.plans = new Backbone.Collection( + data.plans, + { + model: HostingServicePlan, + parse: true, + }); + + return newData; + }, +}); + + +/** + * Information on a configurable SCMTool. + * + * This represents a SCMTool that can be configured, either for a custom + * repository or a hosting service repository. It includes state on the + * capabilities of the SCMTool relevant for configuration. + * + * Version Added: + * 6.0 + * + * Model Attributes: + * isFake (boolean): + * Whether this is a fake (non-operational, information-only) tool + * entry. + * + * name (string): + * The display name of the tool. + * + * supportsPendingChangesets (boolean): + * Whether this tool supports looking up pending changesets. + * + * supportsPostCommit (boolean): + * Whether this tool supports post-commit review. + */ +const SCMToolOption = Backbone.Model.extend({ + defaults: { + isFake: false, + name: null, + supportsPendingChangesets: false, + supportsPostCommit: false, + }, +}); + + +/** + * State for the repository configuration form. + * + * This stores the available tools, hosting services, and accounts available + * for configuring new or existing repositories. + * + * Version Added: + * 6.0 + * + * Attributes: + * hostingServiceOptions (Backbone.Collection of RepositoryHostingService): + * The collection of hosting services available for this repository. + * + * toolOptions (Backbone.Collection of RepositoryTool): + * The collection of SCMTools available for this repository. + */ +RB.RepositoryForm = Backbone.Model.extend({ + /** + * Parse attributes to set for the form. + * + * This will pull out the ``hostingServices`` and ``tools`` attributes + * into new :js:attr:`hostingServices` and :js:attr:`tools` instance-level + * attributes (respectively). + * + * Args: + * data (object): + * The data to parse. + * + * Returns: + * object: + * The parsed attributes to set for the model. + */ + parse(data) { + const optionsData = data.options; + + this.hostingServiceOptions = new Backbone.Collection( + optionsData.hostingServices, + { + model: HostingServiceOption, + parse: true, + }); + + this.toolOptions = new Backbone.Collection( + optionsData.tools, + { + model: SCMToolOption, + parse: true, + }); + + return {}; + }, + + /** + * Return a hosting service option with the given ID. + * + * The ID must be for a valid, registered hosting service. + * + * Args: + * hostingServiceID (string): + * The ID of the hosting service. + * + * Returns: + * HostingServiceOption: + * The hosting service option for the given ID. + */ + getHostingServiceOption(hostingServiceID) { + const hostingServiceOption = + this.hostingServiceOptions.get(hostingServiceID); + + console.assert( + hostingServiceOption, + `"${hostingServiceID}" is not a registered hosting service ID.`); + + return hostingServiceOption; + }, + + /** + * Return a SCMTool option with the given ID. + * + * The ID must be for a valid, registered SCMTool. + * + * Args: + * toolID (string): + * The ID of the SCMTool. + * + * Returns: + * SCMToolOption: + * The SCMTool option for the given ID. + */ + getSCMToolOption(toolID) { + const toolOption = this.toolOptions.get(toolID); + + console.assert( + toolOption, + `"${toolID}" is not a registered SCMTool ID.`); + + return toolOption; + }, +}); diff --git a/reviewboard/static/rb/js/admin/models/tests/repositoryFormModelTests.es6.js b/reviewboard/static/rb/js/admin/models/tests/repositoryFormModelTests.es6.js new file mode 100644 index 0000000000000000000000000000000000000000..57ac7bdd123bec1c50e802bfce9142b922c6adef --- /dev/null +++ b/reviewboard/static/rb/js/admin/models/tests/repositoryFormModelTests.es6.js @@ -0,0 +1,209 @@ +suite('rb/admin/models/RepositoryForm', function() { + describe('Methods', function() { + it('parse', function() { + const form = new RB.RepositoryForm( + { + options: { + hostingServices: [ + { + id: 'service1', + name: 'Service 1', + scmtools: ['tool1', 'Tool 2'], + }, + { + accounts: [ + { + id: 123, + username: 'user1', + }, + { + hostingURL: 'https://example.com', + id: 124, + isAuthorized: true, + username: 'user2', + }, + ], + plans: [ + { + id: 'plan1', + name: 'Plan 1', + }, + { + bugTrackerRequiresUsername: true, + id: 'plan2', + name: 'Plan 2', + }, + ], + id: 'service2', + isFake: true, + isSelfHosted: true, + name: 'Service 2', + needsAuthorization: true, + needsTwoFactorAuthCode: true, + scmtools: ['tool2'], + supportsBugTrackers: true, + supportsSSHKeyAssociation: true, + supportsTwoFactorAuth: true, + }, + + ], + tools: [ + { + id: 'tool1', + name: 'Tool 1', + }, + { + id: 'tool2', + name: 'Tool 2', + isFake: true, + supportsPendingChangesets: true, + supportsPostCommit: true, + }, + ], + }, + }, + { + parse: true, + }); + + expect(form.hostingServiceOptions.length).toBe(2); + expect(form.toolOptions.length).toBe(2); + + /* Check the first hosting service. */ + let hostingService = form.hostingServiceOptions.at(0); + expect(hostingService.id).toBe('service1'); + expect(hostingService.attributes).toEqual({ + id: 'service1', + isFake: false, + isSelfHosted: false, + name: 'Service 1', + needsAuthorization: false, + needsTwoFactorAuthCode: false, + scmtools: ['tool1', 'Tool 2'], + supportsBugTrackers: false, + supportsSSHKeyAssociation: false, + supportsTwoFactorAuth: false, + }); + + expect(hostingService.accounts.length).toBe(0); + expect(hostingService.plans.length).toBe(0); + + /* Check the second hosting service. */ + hostingService = form.hostingServiceOptions.at(1); + expect(hostingService.id).toBe('service2'); + expect(hostingService.attributes).toEqual({ + id: 'service2', + isFake: true, + isSelfHosted: true, + name: 'Service 2', + needsAuthorization: true, + needsTwoFactorAuthCode: true, + scmtools: ['tool2'], + supportsBugTrackers: true, + supportsSSHKeyAssociation: true, + supportsTwoFactorAuth: true, + }); + + expect(hostingService.accounts.length).toBe(2); + + let account = hostingService.accounts.at(0); + expect(account.id).toBe(123); + expect(account.attributes).toEqual({ + hostingURL: null, + id: 123, + isAuthorized: false, + username: 'user1', + }); + + account = hostingService.accounts.at(1); + expect(account.id).toBe(124); + expect(account.attributes).toEqual({ + hostingURL: 'https://example.com', + id: 124, + isAuthorized: true, + username: 'user2', + }); + + expect(hostingService.plans.length).toBe(2); + + let plan = hostingService.plans.at(0); + expect(plan.id).toBe('plan1'); + expect(plan.attributes).toEqual({ + bugTrackerRequiresUsername: false, + id: 'plan1', + name: 'Plan 1', + }); + + plan = hostingService.plans.at(1); + expect(plan.id).toBe('plan2'); + expect(plan.attributes).toEqual({ + bugTrackerRequiresUsername: true, + id: 'plan2', + name: 'Plan 2', + }); + + /* Check the first SCMTool. */ + let tool = form.toolOptions.at(0); + expect(tool.id).toBe('tool1'); + expect(tool.attributes).toEqual({ + id: 'tool1', + name: 'Tool 1', + isFake: false, + supportsPendingChangesets: false, + supportsPostCommit: false, + }); + + /* Check the second SCMTool. */ + tool = form.toolOptions.at(1); + expect(tool.id).toBe('tool2'); + expect(tool.attributes).toEqual({ + id: 'tool2', + name: 'Tool 2', + isFake: true, + supportsPendingChangesets: true, + supportsPostCommit: true, + }); + }); + + it('getHostingServiceOption', function() { + const form = new RB.RepositoryForm( + { + options: { + hostingServices: [ + { + id: 'service1', + name: 'Service 1', + scmtools: ['tool1', 'Tool 2'], + }, + ], + }, + }, + { + parse: true, + }); + + const hostingService = form.getHostingServiceOption('service1'); + expect(hostingService.id).toBe('service1'); + }); + + it('getSCMToolOption', function() { + const form = new RB.RepositoryForm( + { + options: { + tools: [ + { + id: 'tool1', + name: 'Tool 1', + }, + ], + }, + }, + { + parse: true, + }); + + const tool = form.getSCMToolOption('tool1'); + expect(tool.id).toBe('tool1'); + }); + }); +}); diff --git a/reviewboard/static/rb/js/admin/views/repositoryFormView.es6.js b/reviewboard/static/rb/js/admin/views/repositoryFormView.es6.js index e1f98f89b875116c43138c1479866433255904dc..ef2af3210eac4410ed1d172d07741f1d58b84401 100644 --- a/reviewboard/static/rb/js/admin/views/repositoryFormView.es6.js +++ b/reviewboard/static/rb/js/admin/views/repositoryFormView.es6.js @@ -41,13 +41,6 @@ RB.RepositoryFormView = Backbone.View.extend({

`, - /** - * Initialize the form. - */ - initialize() { - this._origRepoTypes = []; - }, - /** * Render the form. * @@ -179,15 +172,6 @@ RB.RepositoryFormView = Backbone.View.extend({ this._associateSSHKeyDisabled = this._$associateSSHKey.prop('disabled'); - this._$tool.find('option').each((i, el) => { - const $repoType = $(el); - - this._origRepoTypes.push({ - value: $repoType.val(), - text: $repoType.text(), - }); - }); - this._onHostingTypeChanged(); this._onRepositoryToolChanged(); this._onBugTrackerTypeChanged(); @@ -209,7 +193,7 @@ RB.RepositoryFormView = Backbone.View.extend({ const hostingType = this._$hostingType.val(); const $hostingAccount = this._$hostingAccount; const $authForm = $(`#hosting-auth-form-${hostingType}`); - const hostingInfo = HOSTING_SERVICES[hostingType]; + const hostingInfo = this.model.getHostingServiceOption(hostingType); const accounts = hostingInfo.accounts; const selectedAccount = parseInt($hostingAccount.val(), 10); let foundSelected = false; @@ -217,7 +201,7 @@ RB.RepositoryFormView = Backbone.View.extend({ /* Rebuild the list of accounts. */ $hostingAccount.find('option[value!=""]').remove(); - if (hostingInfo.needs_two_factor_auth_code || + if (hostingInfo.get('needsTwoFactorAuthCode') || $authForm.find('.errorlist').length > 0) { /* * The first one will be selected automatically, which @@ -226,24 +210,24 @@ RB.RepositoryFormView = Backbone.View.extend({ foundSelected = true; } - accounts.forEach(account => { + for (const account of accounts) { const username = account.get('username'); const hostingURL = account.get('hostingURL'); const $opt = $('