Skip to content

Commit

Permalink
417 score normalization (#432)
Browse files Browse the repository at this point in the history
* fix(api): normalize score naming

* fix(api): use schema resolver instead of manual format

* fix(api): fix default values in test for score response

* fix(api): update historical endpoint to return DetailedScoreResponse and allow filtering by only address
  • Loading branch information
tim-schultz authored Oct 16, 2023
1 parent b9e9b29 commit 47f3dc0
Show file tree
Hide file tree
Showing 13 changed files with 198 additions and 91 deletions.
2 changes: 1 addition & 1 deletion api/aws_lambdas/submit_passport/submit_passport.py
Original file line number Diff line number Diff line change
Expand Up @@ -87,7 +87,7 @@ def handler(_event, _context):
# TODO: preferably we would have a 1:1 mapping of the fields for DetailedScoreResponse
# or if not, then specify a resolver for stamp_scores
ret = DetailedScoreResponse.from_orm(score)
ret.stamp_scores = score.points
ret.stamp_scores = score.stamp_scores

return {
"statusCode": 200,
Expand Down
89 changes: 81 additions & 8 deletions api/registry/api/common.py
Original file line number Diff line number Diff line change
Expand Up @@ -5,13 +5,14 @@

# --- Deduplication Modules
from account.models import Community
from registry.api.utils import ApiKey, check_rate_limit, with_read_db
from registry.api.schema import (
CursorPaginatedHistoricalScoreResponse,
DetailedScoreResponse,
ErrorMessageResponse,
)
from registry.api.utils import ApiKey, check_rate_limit, with_read_db
from registry.exceptions import InvalidLimitException, api_get_object_or_404
from registry.models import Event
from registry.models import Event, Score
from registry.utils import (
decode_cursor,
get_cursor_query_condition,
Expand Down Expand Up @@ -52,7 +53,33 @@ def get_score_history(
else:
created_at = None

# Scenario 1 - Snapshot for 1 addresses
# Scenario 2 - Snapshot for 1 addresses
# the user has passed in the address, but no created_at
# In this case only 1 result will be returned
if address and not created_at:
scores = base_query.filter(address=address).order_by("-created_at")

score_response = []
for score in scores:
score_data = DetailedScoreResponse(
address=score.address,
score=score.data["score"],
status=Score.Status.DONE,
last_score_timestamp=score.created_at.isoformat(),
evidence=score.data["evidence"],
# below aren't currently stored in the events table, but can be
error=None,
stamp_scores=None,
)

score_response.append(score_data)

response = CursorPaginatedHistoricalScoreResponse(
next=None, prev=None, items=score_response
)
return response

# Scenario 2 - Snapshot for 1 addresses and timestamp
# the user has passed in the created_at and address
# In this case only 1 result will be returned
if address and created_at:
Expand All @@ -62,14 +89,30 @@ def get_score_history(
.first()
)

if not score:
return CursorPaginatedHistoricalScoreResponse(
next=None, prev=None, items=[]
)

score.created_at = score.created_at.isoformat()

score_data = DetailedScoreResponse(
address=address,
score=score.data["score"],
status=Score.Status.DONE,
last_score_timestamp=score.created_at,
evidence=score.data["evidence"],
# below aren't currently stored in the events table, but can be
error=None,
stamp_scores=None,
)

response = CursorPaginatedHistoricalScoreResponse(
next=None, prev=None, items=[score]
next=None, prev=None, items=[score_data]
)
return response

# Scenario 2 - Snapshot for all addresses
# Scenario 3 - Snapshot for all addresses
# the user has passed in the created_at, but no address
elif created_at:
pagination_sort_fields = ["address"]
Expand Down Expand Up @@ -103,12 +146,27 @@ def get_score_history(
endpoint,
)

score_response = []
for score in scores:
score_data = DetailedScoreResponse(
address=score.address,
score=score.data["score"],
status=Score.Status.DONE,
last_score_timestamp=score.created_at,
evidence=score.data["evidence"],
# below aren't currently stored in the events table, but can be
error=None,
stamp_scores=None,
)

score_response.append(score_data)

response = CursorPaginatedHistoricalScoreResponse(
next=page_links["next"], prev=page_links["prev"], items=scores
next=page_links["next"], prev=page_links["prev"], items=score_response
)

return response
# Scenario 3 - Just return history ...
# # Scenario 4 - Just return history ...
else:
pagination_sort_fields = ["id"]
filter_condition, field_ordering = get_cursor_query_condition(
Expand Down Expand Up @@ -141,8 +199,23 @@ def get_score_history(
endpoint,
)

score_response = []
for score in scores:
score_data = DetailedScoreResponse(
address=score.address,
score=score.data["score"],
status=Score.Status.DONE,
last_score_timestamp=score.created_at,
evidence=score.data["evidence"],
# below aren't currently stored in the events table, but can be
error=None,
stamp_scores=None,
)

score_response.append(score_data)

response = CursorPaginatedHistoricalScoreResponse(
next=page_links["next"], prev=page_links["prev"], items=scores
next=page_links["next"], prev=page_links["prev"], items=score_response
)
return response

Expand Down
8 changes: 7 additions & 1 deletion api/registry/api/schema.py
Original file line number Diff line number Diff line change
Expand Up @@ -92,6 +92,12 @@ def resolve_last_score_timestamp(obj):
def resolve_address(obj):
return obj.passport.address

@staticmethod
def resolve_stamp_scores(obj):
if obj.stamp_scores is None or obj.stamp_scores == "":
return {}
return obj.stamp_scores


class HistoricalScoreData(Schema):
score: float
Expand All @@ -116,7 +122,7 @@ class CursorPaginatedScoreResponse(Schema):
class CursorPaginatedHistoricalScoreResponse(Schema):
next: Optional[str]
prev: Optional[str]
items: List[DetailedHistoricalScoreResponse]
items: List[DetailedScoreResponse]


class SimpleScoreResponse(Schema):
Expand Down
15 changes: 1 addition & 14 deletions api/registry/api/v1.py
Original file line number Diff line number Diff line change
Expand Up @@ -385,20 +385,7 @@ def handle_get_score(
score = Score.objects.get(
passport__address=lower_address, passport__community=user_community
)

response_data = {
"address": score.passport.address,
"score": score.score,
"status": score.status,
"last_score_timestamp": score.last_score_timestamp.isoformat()
if score.last_score_timestamp
else None,
"evidence": score.evidence,
"error": score.error,
"stamp_scores": score.points,
}

return DetailedScoreResponse(**response_data)
return score
except NotFoundApiException as e:
raise e
except Exception as e:
Expand Down
2 changes: 1 addition & 1 deletion api/registry/atasks.py
Original file line number Diff line number Diff line change
Expand Up @@ -63,7 +63,7 @@ async def acalculate_score(passport: Passport, community_id: int, score: Score):
score.last_score_timestamp = get_utc_time()
score.evidence = scoreData.evidence[0].as_dict() if scoreData.evidence else None
score.error = None
score.points = scoreData.points
score.stamp_scores = scoreData.stamp_scores
log.info("Calculated score: %s", score)


Expand Down
4 changes: 2 additions & 2 deletions api/registry/management/commands/recalculate_scores.py
Original file line number Diff line number Diff line change
Expand Up @@ -113,7 +113,7 @@ def handle(self, *args, **kwargs):
else None
)
score.error = None
score.points = scoreData.points
score.stamp_scores = scoreData.stamp_scores

if scores_to_create:
Score.objects.bulk_create(scores_to_create)
Expand All @@ -127,7 +127,7 @@ def handle(self, *args, **kwargs):
"last_score_timestamp",
"evidence",
"error",
"points",
"stamp_scores",
],
)

Expand Down
18 changes: 18 additions & 0 deletions api/registry/migrations/0025_rename_points_score_stamp_scores.py
Original file line number Diff line number Diff line change
@@ -0,0 +1,18 @@
# Generated by Django 4.2.4 on 2023-10-13 20:01

from django.db import migrations


class Migration(migrations.Migration):

dependencies = [
("registry", "0022_stamp_points_squashed_0024_alter_score_points"),
]

operations = [
migrations.RenameField(
model_name="score",
old_name="points",
new_name="stamp_scores",
),
]
21 changes: 10 additions & 11 deletions api/registry/models.py
Original file line number Diff line number Diff line change
Expand Up @@ -70,7 +70,7 @@ class Status:
)
error = models.TextField(null=True, blank=True)
evidence = models.JSONField(null=True, blank=True)
points = models.JSONField(null=True, blank=True)
stamp_scores = models.JSONField(null=True, blank=True)

def __str__(self):
return f"Score #{self.id}, score={self.score}, last_score_timestamp={self.last_score_timestamp}, status={self.status}, error={self.error}, evidence={self.evidence}, passport_id={self.passport_id}"
Expand All @@ -81,16 +81,15 @@ def score_updated(sender, instance, **kwargs):
if instance.status != Score.Status.DONE:
return instance

if not Score.objects.filter(pk=instance.pk, score=instance.score).exists():
Event.objects.create(
action=Event.Action.SCORE_UPDATE,
address=instance.passport.address,
community=instance.passport.community,
data={
"score": float(instance.score) if instance.score != None else 0,
"evidence": instance.evidence,
},
)
Event.objects.create(
action=Event.Action.SCORE_UPDATE,
address=instance.passport.address,
community=instance.passport.community,
data={
"score": float(instance.score) if instance.score != None else 0,
"evidence": instance.evidence,
},
)

return instance

Expand Down
54 changes: 27 additions & 27 deletions api/registry/test/test_command_recalculate_scores.py
Original file line number Diff line number Diff line change
Expand Up @@ -178,21 +178,21 @@ def test_rescoring_binary_scorer(

s1 = Score.objects.get(passport=binary_weighted_scorer_passports[0])
assert s1.evidence["rawScore"] == "75"
assert len(s1.points) == 1
assert "Facebook" in s1.points
assert len(s1.stamp_scores) == 1
assert "Facebook" in s1.stamp_scores

s2 = Score.objects.get(passport=binary_weighted_scorer_passports[1])
assert s2.evidence["rawScore"] == "76"
assert len(s2.points) == 2
assert "Facebook" in s2.points
assert "Google" in s2.points
assert len(s2.stamp_scores) == 2
assert "Facebook" in s2.stamp_scores
assert "Google" in s2.stamp_scores

s3 = Score.objects.get(passport=binary_weighted_scorer_passports[2])
assert s3.evidence["rawScore"] == "77"
assert len(s3.points) == 3
assert "Facebook" in s3.points
assert "Google" in s3.points
assert "Ens" in s3.points
assert len(s3.stamp_scores) == 3
assert "Facebook" in s3.stamp_scores
assert "Google" in s3.stamp_scores
assert "Ens" in s3.stamp_scores

def test_rescoring_weighted_scorer(
self,
Expand Down Expand Up @@ -224,21 +224,21 @@ def test_rescoring_weighted_scorer(

s1 = Score.objects.get(passport=weighted_scorer_passports[0])
assert s1.score == 1
assert len(s1.points) == 1
assert "Facebook" in s1.points
assert len(s1.stamp_scores) == 1
assert "Facebook" in s1.stamp_scores

s2 = Score.objects.get(passport=weighted_scorer_passports[1])
assert s2.score == 2
assert len(s2.points) == 2
assert "Facebook" in s2.points
assert "Google" in s2.points
assert len(s2.stamp_scores) == 2
assert "Facebook" in s2.stamp_scores
assert "Google" in s2.stamp_scores

s3 = Score.objects.get(passport=weighted_scorer_passports[2])
assert s3.score == 3
assert len(s3.points) == 3
assert "Facebook" in s3.points
assert "Google" in s3.points
assert "Ens" in s3.points
assert len(s3.stamp_scores) == 3
assert "Facebook" in s3.stamp_scores
assert "Google" in s3.stamp_scores
assert "Ens" in s3.stamp_scores

#########################################################
# Change weights and rescore ...
Expand All @@ -259,21 +259,21 @@ def test_rescoring_weighted_scorer(

s1 = Score.objects.get(passport=weighted_scorer_passports[0])
assert s1.score == 75
assert len(s1.points) == 1
assert "Facebook" in s1.points
assert len(s1.stamp_scores) == 1
assert "Facebook" in s1.stamp_scores

s2 = Score.objects.get(passport=weighted_scorer_passports[1])
assert s2.score == 76
assert len(s2.points) == 2
assert "Facebook" in s2.points
assert "Google" in s2.points
assert len(s2.stamp_scores) == 2
assert "Facebook" in s2.stamp_scores
assert "Google" in s2.stamp_scores

s3 = Score.objects.get(passport=weighted_scorer_passports[2])
assert s3.score == 77
assert len(s3.points) == 3
assert "Facebook" in s3.points
assert "Google" in s3.points
assert "Ens" in s3.points
assert len(s3.stamp_scores) == 3
assert "Facebook" in s3.stamp_scores
assert "Google" in s3.stamp_scores
assert "Ens" in s3.stamp_scores

def test_rescoring_include_filter(
self,
Expand Down
6 changes: 3 additions & 3 deletions api/registry/test/test_passport_submission.py
Original file line number Diff line number Diff line change
Expand Up @@ -542,7 +542,7 @@ def test_submitting_without_passport(self, aget_passport, validate_credential):
"last_score_timestamp": None,
"status": "ERROR",
"error": "No Passport found for this address.",
"stamp_scores": None,
"stamp_scores": {},
}

@patch("registry.atasks.validate_credential", side_effect=[[], [], [], []])
Expand Down Expand Up @@ -576,7 +576,7 @@ def test_submit_passport_multiple_times(
"score": Decimal("2.000000000"),
"status": "DONE",
"error": None,
"stamp_scores": None,
"stamp_scores": {"Ens": 1.0, "Google": 1.0},
}

expected2ndResponse = {
Expand All @@ -586,7 +586,7 @@ def test_submit_passport_multiple_times(
"score": Decimal("2.000000000"),
"status": "DONE",
"error": None,
"stamp_scores": None,
"stamp_scores": {"Ens": 1.0, "Google": 1.0},
}

# First submission
Expand Down
Loading

0 comments on commit 47f3dc0

Please sign in to comment.