Skip to content

Commit

Permalink
Addressed comments 2
Browse files Browse the repository at this point in the history
  • Loading branch information
aksareen committed Jul 31, 2017
1 parent 714dbca commit 7429202
Show file tree
Hide file tree
Showing 5 changed files with 24 additions and 38 deletions.
11 changes: 5 additions & 6 deletions auslib/web/admin/views/permissions.py
Original file line number Diff line number Diff line change
Expand Up @@ -151,11 +151,13 @@ def __init__(self):
@requirelogin
def _post(self, transaction, changed_by):
change_type = connexion.request.json.get("change_type")
connexion.request.json.pop("csrf_token", None)

what = {}
for field in connexion.request.json:
if change_type == "delete" and field == "options" or change_type == "insert" and field == "data_version":
# TODO: currently UI passes extra field(options) in change_type == 'delete' request body. Fix it and
# TODO: change the below operation from filter/pop to throw Error when extra field is passed.
if (field == "csrf_token" or (change_type == "delete" and field == "options") or
(change_type == "insert" and field == "data_version")):
continue

what[field] = connexion.request.json[field]
Expand All @@ -169,10 +171,7 @@ def _post(self, transaction, changed_by):
else:
what["data_version"] = int(what["data_version"])

elif change_type != "insert":
return problem(400, "Bad Request", "Invalid or missing change_type")

return super(PermissionScheduledChangesView, self)._post(what, transaction, changed_by)
return super(PermissionScheduledChangesView, self)._post(what, transaction, changed_by, change_type)


class PermissionScheduledChangeView(ScheduledChangeView):
Expand Down
8 changes: 2 additions & 6 deletions auslib/web/admin/views/releases.py
Original file line number Diff line number Diff line change
Expand Up @@ -517,11 +517,10 @@ def __init__(self):
@requirelogin
def _post(self, transaction, changed_by):
change_type = connexion.request.json.get("change_type")
connexion.request.json.pop("csrf_token", None)

what = {}
for field in connexion.request.json:
if change_type == "insert" and field == "data_version":
if field == "csrf_token" or change_type == "insert" and field == "data_version":
continue
what[field] = connexion.request.json[field]

Expand All @@ -545,10 +544,7 @@ def _post(self, transaction, changed_by):
if not what.get("data_version", None):
return problem(400, "Bad Request", "Missing field", ext={"exception": "data_version is missing"})

else:
return problem(400, "Bad Request", "Invalid or missing change_type")

return super(ReleaseScheduledChangesView, self)._post(what, transaction, changed_by)
return super(ReleaseScheduledChangesView, self)._post(what, transaction, changed_by, change_type)


class ReleaseScheduledChangeView(ScheduledChangeView):
Expand Down
20 changes: 6 additions & 14 deletions auslib/web/admin/views/required_signoffs.py
Original file line number Diff line number Diff line change
Expand Up @@ -118,11 +118,10 @@ def __init__(self):
@requirelogin
def _post(self, transaction, changed_by):
change_type = connexion.request.json.get("change_type")
connexion.request.json.pop("csrf_token", None)

what = {}
for field in connexion.request.json:
if change_type == "insert" and field == "data_version":
if field == "csrf_token" or change_type == "insert" and field == "data_version":
continue
what[field] = connexion.request.json[field]

Expand All @@ -145,11 +144,8 @@ def _post(self, transaction, changed_by):
else:
what["data_version"] = int(what["data_version"])

else:
self.log.warning("Bad input: %s", change_type)
return problem(400, "Bad Request", "Invalid or missing change_type")

return super(ProductRequiredSignoffsScheduledChangesView, self)._post(what, transaction, changed_by)
return super(ProductRequiredSignoffsScheduledChangesView, self)._post(what, transaction, changed_by,
change_type)


class ProductRequiredSignoffScheduledChangeView(ScheduledChangeView):
Expand Down Expand Up @@ -232,11 +228,10 @@ def __init__(self):
@requirelogin
def _post(self, transaction, changed_by):
change_type = connexion.request.json.get("change_type")
connexion.request.json.pop("csrf_token", None)

what = {}
for field in connexion.request.json:
if change_type == "insert" and field == "data_version":
if field == "csrf_token" or change_type == "insert" and field == "data_version":
continue
what[field] = connexion.request.json[field]

Expand All @@ -259,11 +254,8 @@ def _post(self, transaction, changed_by):
else:
what["data_version"] = int(what["data_version"])

else:
self.log.warning("Bad input: %s", change_type)
return problem(400, "Bad Request", "Invalid or missing change_type")

return super(PermissionsRequiredSignoffsScheduledChangesView, self)._post(what, transaction, changed_by)
return super(PermissionsRequiredSignoffsScheduledChangesView, self)._post(what, transaction, changed_by,
change_type)


class PermissionsRequiredSignoffScheduledChangeView(ScheduledChangeView):
Expand Down
18 changes: 7 additions & 11 deletions auslib/web/admin/views/rules.py
Original file line number Diff line number Diff line change
Expand Up @@ -295,10 +295,14 @@ def _post(self, transaction, changed_by):
else:
change_type = connexion.request.values.get("change_type")

connexion.request.json.pop("csrf_token", None)
what = {}
delete_change_type_allowed_fields = ["telemetry_product", "telemetry_channel", "telemetry_uptake", "when",
"rule_id", "data_version", "change_type"]
for field in connexion.request.json:
if change_type == "insert" and field in ["rule_id", "data_version"]:
# TODO: currently UI passes extra rule model fields in change_type == 'delete' request body. Fix it and
# TODO: change the below operation from filter/pop to throw Error when extra fields are passed.
if (field == "csrf_token" or (change_type == "insert" and field in ["rule_id", "data_version"]) or
(change_type == "delete" and field not in delete_change_type_allowed_fields)):
continue

if field in ["rule_id", "data_version"]:
Expand Down Expand Up @@ -328,15 +332,7 @@ def _post(self, transaction, changed_by):
if what.get('fallbackMapping') is not None and len(fallback_mapping_values) != 1:
return problem(400, 'Bad Request', 'Invalid fallbackMapping value. No release name found in DB')

# TODO: currently UI passes extra rule model fields in change_type == 'delete' request body. Fix it and
# TODO: change the below operation from filter/pop to throw Error when extra fields are passed.
elif change_type == "delete":
for key in list(what):
if key not in ["telemetry_product", "telemetry_channel", "telemetry_uptake", "when", "rule_id",
"data_version", "change_type"]:
what.pop(key, None)

return super(RuleScheduledChangesView, self)._post(what, transaction, changed_by)
return super(RuleScheduledChangesView, self)._post(what, transaction, changed_by, change_type)


class RuleScheduledChangeView(ScheduledChangeView):
Expand Down
5 changes: 4 additions & 1 deletion auslib/web/admin/views/scheduled_changes.py
Original file line number Diff line number Diff line change
Expand Up @@ -65,7 +65,10 @@ def get(self):
ret["scheduled_changes"].append(scheduled_change)
return jsonify(ret)

def _post(self, what, transaction, changed_by):
def _post(self, what, transaction, changed_by, change_type):
if change_type not in ["insert", "update", "delete"]:
return problem(400, "Bad Request", "Invalid or missing change_type")

if is_when_present_and_in_past_validator(what):
return problem(400, "Bad Request", "Changes may not be scheduled in the past")

Expand Down

0 comments on commit 7429202

Please sign in to comment.