Fix addDeveloperProduct & checkDeveloperProductName. addGamepass feature #785
Conversation
There was a problem hiding this comment.
Thank you for your PR, I've highlighted a few issues.
Your changes to checkDeveloperProductName would be breaking changes but we intend to move to a new major version - so that isn't an issue. I agree with your API changes there; returning a boolean is the best API.
I am not comfortable adding a dependency for form data as I believe it is done manually elsewhere and there are built in APIs for doing so (it is url encoded I believe so URLSearchParams could work)
| data.forEach(product => { | ||
| if(product.name == args.productName || product.id == productId){ | ||
| exists = true | ||
| return |
There was a problem hiding this comment.
This returns undefined and the typing says it returns a boolean.
Would a better approach be:
for (const product of data) {
if (...) {
return true
}
}
return false
```?| @@ -23,17 +23,25 @@ exports.func = (args) => { | |||
| const productId = parseInt(args.productId) ? parseInt(args.productId) : 0 | |||
|
|
|||
| return http({ | |||
| url: '//www.roblox.com/places/check-developerproduct-name?universeId=' + universeId + '&developerProductId=' + productId + '&developerProductName=' + args.productName + '&_=1515792139751', | |||
| url: `//apis.roblox.com/developer-products/v1/universes/${universeId}/developerproducts?pageNumber=1&pageSize=2147483647`, | |||
There was a problem hiding this comment.
What is the meaning of the page size here? Why is it a large arbitrary number? how does this work when the number of pages is greater than 1? Could this use a pagination wrapper
There was a problem hiding this comment.
Per my testing, Roblox does not use cursors for this endpoint. The number specified for the page size is (2^31)-1 (the largest possible signed 32bit int), which happens to be the largest page size Roblox supports. Now, whether the api will actually return that many results, I don't know.
I agree that a pagination wrapper would be preferable to this though, given this is already undocumented to start with and I don't really expect this to work in practice (but maybe somehow it does).
| **/ | ||
|
|
||
| const nextFunction = (jar, token, universeId, name, priceInRobux, description) => { | ||
| const FormData = require('form-data'); |
There was a problem hiding this comment.
Don't require modules in functions like this.
This does not need a new dependency - I believe the new fetch API has a built in way of building these, and they are built manually elsewhere in the library.
| resolveWithFullResponse: true | ||
| } | ||
| }).then((res) => { | ||
| console.log(res) |
| @@ -59,7 +59,7 @@ const nextFunction = (jar, token, universeId, name, priceInRobux, description) = | |||
| exports.func = (args) => { | |||
| const jar = args.jar | |||
|
|
|||
| return getGeneralToken({ jar }).then((xcsrf) => { | |||
| return getGeneralToken({ jar: jar }).then((xcsrf) => { | |||
There was a problem hiding this comment.
Not sure what the point of this change is
|
Adding help wanted if anyone wants to help out and resolve the above |
|
Just seen this pr after I posted a fix for the same function #806 |
|
Closing this as it is stale. Please open a new PR if above are fixed |
Fixed addDeveloperProduct & checkDeveloperProductName. Added 'addGamepass' function.