Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
2 changes: 1 addition & 1 deletion migrations_lockfile.txt
Original file line number Diff line number Diff line change
Expand Up @@ -10,7 +10,7 @@ auth: 0008_alter_user_username_max_length
contenttypes: 0002_remove_content_type_name
jira_ac: 0001_initial
nodestore: 0001_initial
sentry: 0095_ruleactivity
sentry: 0096_sentry_app_component_skip_load_on_open
sessions: 0001_initial
sites: 0002_alter_domain_unique
social_auth: 0001_initial
2 changes: 2 additions & 0 deletions src/sentry/api/validators/sentry_apps/schema.py
Original file line number Diff line number Diff line change
Expand Up @@ -44,6 +44,8 @@
"type": {"type": "string", "enum": ["select"]},
"label": {"type": "string"},
"name": {"type": "string"},
"async": {"type": "boolean"},
Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

do you think we could add these options in the test_issue_link test?

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

@MeredithAnya Which test is this? can't find it.

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

validators/sentry_apps/test_issue_link.py p sure

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

@MeredithAnya My bad, I didn't realize it was a file. Yes, I will update it.

"skip_load_on_open": {"type": "boolean"},
"uri": {"$ref": "#/definitions/uri"},
"options": {"$ref": "#/definitions/options"},
"depends_on": {"type": "array", "minItems": 1, "items": {"type": "string"}},
Expand Down
2 changes: 1 addition & 1 deletion src/sentry/mediators/sentry_app_components/preparer.py
Original file line number Diff line number Diff line change
Expand Up @@ -54,7 +54,7 @@ def _prepare_field(self, field):
field.update({"choices": field["options"]})

if "uri" in field:
if "async" not in field:
if not field.get("skip_load_on_open"):
field.update(self._request(field["uri"]))

def _request(self, uri):
Expand Down
Original file line number Diff line number Diff line change
@@ -0,0 +1,73 @@
# -*- coding: utf-8 -*-
# Generated by Django 1.11.29 on 2020-07-28 22:38
from __future__ import unicode_literals

from django.db import migrations

#update the field with mutation
def convert_field(field):
# even if async if false, we had a bug where we'd treat it the same as true
# so to maintain legacy behavior, we have to replicate that same check when setting skip_load_on_open
if "async" in field:
field["skip_load_on_open"] = True
del field["async"]


# updates the schema with mutation
def update_element_schema(schema):
# update all the fields in the schema
link = schema.get("link", {})
create = schema.get("create", {})

for field in link.get("required_fields", []):
convert_field(field)

for field in link.get("optional_fields", []):
convert_field(field)

for field in create.get("required_fields", []):
convert_field(field)

for field in create.get("optional_fields", []):
convert_field(field)

def update_ui_components(apps, schema_editor):
SentryAppComponent = apps.get_model("sentry", "SentryAppComponent")
for component in SentryAppComponent.objects.filter(type="issue-link").select_related("sentry_app"):
# need to update the denormalized data
update_element_schema(component.schema)
for element in component.sentry_app.schema.get("elements", []):
# only update issue link elements
if element.get("type") == "issue-link":
update_element_schema(element)

# save the UI component and the sentry app
component.save()
component.sentry_app.save()



class Migration(migrations.Migration):
# This flag is used to mark that a migration shouldn't be automatically run in
# production. We set this to True for operations that we think are risky and want
# someone from ops to run manually and monitor.
# General advice is that if in doubt, mark your migration as `is_dangerous`.
# Some things you should always mark as dangerous:
# - Large data migrations. Typically we want these to be run manually by ops so that
# they can be monitored. Since data migrations will now hold a transaction open
# this is even more important.
# - Adding columns to highly active tables, even ones that are NULL.
is_dangerous = False

# This flag is used to decide whether to run this migration in a transaction or not.
# By default we prefer to run in a transaction, but for migrations where you want
# to `CREATE INDEX CONCURRENTLY` this needs to be set to False. Typically you'll
# want to create an index concurrently when adding one to an existing table.
atomic = False


dependencies = [
('sentry', '0095_ruleactivity'),
]

operations = [migrations.RunPython(update_ui_components, migrations.RunPython.noop)]
Original file line number Diff line number Diff line change
Expand Up @@ -27,6 +27,7 @@ type FieldFromSchema = Omit<Field, 'choices' | 'type'> & {
uri?: string;
depends_on?: string[];
choices?: Array<[any, string]>;
async?: boolean;
};

type Config = {
Expand Down Expand Up @@ -292,7 +293,7 @@ export class SentryAppExternalIssueForm extends React.Component<Props, State> {
const extraProps = field.uri
? {
loadOptions: (input: string) => this.getOptions(field, input),
async: true, //TODO: make configurable
async: typeof field.async === 'undefined' ? true : field.async, //default to true
cache: false,
onSelectResetsInput: false,
onCloseResetsInput: false,
Expand Down
23 changes: 23 additions & 0 deletions tests/sentry/api/validators/sentry_apps/test_issue_link.py
Original file line number Diff line number Diff line change
Expand Up @@ -27,6 +27,19 @@ def setUp(self):
"required_fields": [
{"type": "text", "name": "title", "label": "Title"},
{"type": "text", "name": "description", "label": "Description"},
{
"type": "select",
"uri": "/sentry/tasks/projects",
"name": "project_id",
"label": "Project",
},
{
"depends_on": ["project_id"],
"type": "select",
"uri": "/sentry/tasks/boards",
"name": "board_id",
"label": "Board",
},
],
"optional_fields": [{"type": "text", "name": "owner", "label": "Owner"}],
},
Expand Down Expand Up @@ -87,3 +100,13 @@ def test_missing_link_required_fields(self):
def test_missing_link_optional_fields(self):
del self.schema["link"]["optional_fields"]
validate_component(self.schema)

@invalid_schema
def test_invalid_async_option(self):
self.schema["create"]["required_fields"][2]["async"] = "cat"
validate_component(self.schema)

@invalid_schema
def test_invalid_skip_load_on_open_option(self):
self.schema["create"]["required_fields"][2]["skip_load_on_open"] = "cat"
validate_component(self.schema)
13 changes: 11 additions & 2 deletions tests/sentry/mediators/sentry_app_components/test_preparer.py
Original file line number Diff line number Diff line change
Expand Up @@ -39,7 +39,13 @@ def test_prepares_components_requiring_requests(self, run):
{"type": "select", "name": "bar", "label": "Bar", "uri": "/sentry/bar"}
],
"optional_fields": [
{"type": "select", "name": "baz", "label": "Baz", "uri": "/sentry/baz"}
{
"type": "select",
"name": "baz",
"label": "Baz",
"uri": "/sentry/baz",
"skip_load_on_open": True,
}
],
},
}
Expand All @@ -54,7 +60,10 @@ def test_prepares_components_requiring_requests(self, run):

assert call(install=self.install, project=self.project, uri="/sentry/bar") in run.mock_calls

assert call(install=self.install, project=self.project, uri="/sentry/baz") in run.mock_calls
assert (
not call(install=self.install, project=self.project, uri="/sentry/baz")
in run.mock_calls
)


class TestPreparerStacktraceLink(TestCase):
Expand Down