feat: retrieving user's purchased games

achievements-app / psn-api

A JavaScript library that lets you get trophy, user, and game data from the PlayStation Network.

https://psn-api.achievements.app

MIT License

268 stars 32 forks source link

feat: retrieving user's purchased games #136

Closed alekpentchev closed 1 year ago

alekpentchev commented 1 year ago

I think I'm ready to submit my first PR to this repo. And because of that I'm quite sure there might be some things worth changing. So here I'm counting on your support :) Few notes I want to make:

I tried to follow the conventions from other existing functions
I have extracted a lot of the GraphQL query variables to be available as params, e.g. size of the returned games list etc. This is not only for performance reasons if someone wouldn't like to fetch hundreds of games at once
The URL the code generates seems to be working properly
I had a problem with finding meaningful values for the conceptId property. For every game I got a null value. That's why for now I've added type definition as null | unknown
I'm not sure if having base URLs in separate files is the best approach. What do you think about keeping them in one file which is used to import the URLs from?

Feel free to comment everything. I'm open for every discussion and recommendation.

vercel[bot] commented 1 year ago

The latest updates on your projects. Learn more about Vercel for Git ↗︎

Name	Status	Preview	Comments	Updated
psn-api	✅ Ready (Inspect)	Visit Preview	💬 Add your feedback	Mar 29, 2023 at 9:53AM (UTC)

wescopeland commented 1 year ago

Thank you for submitting a PR! I will not have time to do a comprehensive review until Monday, but rest assured this will not get buried.

Just skimming over the PR, we will also need:

Updates to the documentation website. To start the docs site:

cd website
yarn
yarn serve

An update to README.md that points to the new documentation page.

~~I haven't dug in to why CI is not properly running on GitHub Actions. Before I review on Monday, be sure to double check that yarn verify succeeds.~~

wescopeland commented 1 year ago

Hi @alekpentchev ! Before I continue with a review, I want to double check that this work is not a duplicate of https://github.com/achievements-app/psn-api/pull/134

At first glance, this seems very similar.

alekpentchev commented 1 year ago

Hi @alekpentchev ! Before I continue with a review, I want to double check that this work is not a duplicate of #134

At first glance, this seems very similar.

Yeah, it looks similar. But both these operations serve a bit different purpose. The getUserGameList query shows all games the user has interacted with (not necessarily purchased), while getPurchasedGameList shows only games the user has actually bought or downloaded.

I wasn't aware that the library has this getUserGameList query being used. So probably I should rename my request to not confuse the users.

alekpentchev commented 1 year ago

Added one more commit that changes the naming of my vars, objects etc. to avoid further confusion. Earlier I didn't notice that there's a similar looking function for getting recently played games.

One more difference I noticed is that for the getPurchasedGameList operation there's a bit more we get as a response than in the getUserGameList (used to fetch recently played games).

Please let me know if we're ok with it and if so I'll adjust the docs for this feature.

wescopeland commented 1 year ago

Added one more commit that changes the naming of my vars, objects etc. to avoid further confusion. Earlier I didn't notice that there's a similar looking function for getting recently played games.

One more difference I noticed is that for the getPurchasedGameList operation there's a bit more we get as a response than in the getUserGameList (used to fetch recently played games).

Please let me know if we're ok with it and if so I'll adjust the docs for this feature.

No problem, we will work together to make this the best it can be. I've left some initial feedback and we'll iterate further.

alekpentchev commented 1 year ago

Added some changes based on the comments:

reused GRAPHQL_BASE_URL instead of duplicating this variable
registered the function in the module's index.ts file
double checked if it works in the __playground.ts file
moved my functions to the graphql folder in order to co-locate the GraphQL-related code
changed functions names from getUserPurchasedGames to getPurchasedGames to be clearer when it comes to its meaning

wescopeland commented 1 year ago

Hi! This PR has exceeded 30 days of inactivity. I will be closing it as a courtesy. Please feel free to re-open it at any time you would like to revisit this work.