Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Historical Score Endpoint Introduction #2956

Closed
1 task
erichfi opened this issue Oct 8, 2024 · 12 comments · Fixed by passportxyz/passport-scorer#704
Closed
1 task

Historical Score Endpoint Introduction #2956

erichfi opened this issue Oct 8, 2024 · 12 comments · Fixed by passportxyz/passport-scorer#704
Assignees

Comments

@erichfi
Copy link
Collaborator

erichfi commented Oct 8, 2024

User Story:

As a developer,
I want to only allow partners with explicit permission to access the historical score endpoint,
So that I can ensure secure and controlled usage of this feature.

Acceptance Criteria

GIVEN a request for historical scores,
WHEN the request is made by an allowlisted API key,
THEN access should be granted to the endpoint.

Product & Design Links:

N/A

Tech Details:

  • Officially introduce the historical endpoint for allowlisted API keys.
  • Remove the skip_refresh parameter from all endpoints.

Open Questions:

  • What specific criteria will be used to allowlist partners?

Notes/Assumptions:

  • There may be bugs that need fixing before the public launch.
  • This endpoint should return a user's score at a specific point in time.
@erichfi erichfi moved this to Prioritized in Passport New Oct 8, 2024
@nutrina
Copy link
Collaborator

nutrina commented Oct 14, 2024

Some tech details / hints:

@lebraat FYI on the pagination

@lebraat
Copy link
Member

lebraat commented Oct 16, 2024

My vote -- we stay consistent with the following:
https://docs.passport.xyz/building-with-passport/passport-api/api-reference#pagination

@tim-schultz tim-schultz self-assigned this Oct 17, 2024
@tim-schultz tim-schultz moved this from Prioritized to In Progress (WIP) in Passport New Oct 17, 2024
@tim-schultz
Copy link
Collaborator

@lebraat how should we communicate to existing integratoooors that this endpoint requires special privileges?

@lebraat
Copy link
Member

lebraat commented Oct 21, 2024

Let's include this paragraph within the API Playground:

To access this endpoint, you must submit your use case and be approved by the Passport team. To do so, please fill out the following form, making sure to provide a detailed description of your use case. The Passport team typically reviews and responds to form responses within 48 hours.
https://forms.gle/4GyicBfhtHW29eEu8

@tim-schultz
Copy link
Collaborator

tim-schultz commented Oct 22, 2024

Let's include this paragraph within the API Playground:

To access this endpoint, you must submit your use case and be approved by the Passport team. To do so, please fill out the following form, making sure to provide a detailed description of your use case. The Passport team typically reviews and responds to form responses within 48 hours. https://forms.gle/4GyicBfhtHW29eEu8

Sweet thanks!


@lebraat / @nutrina one more question. There currently is logic to pull snapshots or scores from a single day. Here is a description of the logic we have currently. Since the address is part of the url, I imagine we should remove scenario #3, but should we maintain or update the remaining logic?

Scenario 1 - Snapshot for 1 addresses
the user has passed in the address, but no created_at
In this case only 1 result will be returned

Scenario 2 - Snapshot for 1 address and timestamp
the user has passed in the created_at and address
In this case only 1 result will be returned

Scenario 3 - Snapshot for all addresses
the user has passed in the created_at, but no address

@nutrina
Copy link
Collaborator

nutrina commented Oct 22, 2024

@tim-schultz we only need the history for 1 address: /v2/stamps/{scorer_id}/score/{address}/history

Please double-check this in the doc from @lebraat

@lebraat
Copy link
Member

lebraat commented Oct 22, 2024

Correct.

We only want one scenario moving forward:
Return the score object for the specified user at the specified time

@tim-schultz
Copy link
Collaborator

Correct.

We only want one scenario moving forward: Return the score object for the specified user at the specified time

Gotcha. So if they pass a timestamp it should return a snapshot of the user's score at that time. Otherwise it will return paginated results of the scores over time

@lebraat
Copy link
Member

lebraat commented Oct 22, 2024

We should make the timestamp required and that should be the only function of this endpoint.

We don't want to deliver paginated results of scores over time.

@tim-schultz
Copy link
Collaborator

Alrighty removing pagination and more than one score in the results.

Leaving this commit here for engineering in case we revisit this in the future. I fixed some of the pagination logic and added tests

@tim-schultz
Copy link
Collaborator

@lebraat last question :) Shall we deprecate the v1 historical endpoint since we now require special privileges to access this data?

@lebraat
Copy link
Member

lebraat commented Oct 23, 2024

We can fully retire it. We never formally announced it, so no one should be using it.

Thanks, Tim!

@tim-schultz tim-schultz moved this from Code Complete to In Progress (WIP) in Passport New Oct 24, 2024
@erichfi erichfi moved this from Ready to Deploy to Done in Passport New Nov 4, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
Status: Done
Development

Successfully merging a pull request may close this issue.

4 participants