Mephiles / torntools_extension

A browser extension for Torn.com
GNU General Public License v3.0
109 stars 61 forks source link

Changing to new Nuclear Family API backend for Revive Requests #800

Closed Fogest closed 3 months ago

Fogest commented 3 months ago

Hey, I am currently the main developer for the Nuclear Family now. I have been working to replace our old reviving system and have made a completely new site and API.

It's really just some minor changes swapping the URLs and the names of a few parameters. So hopefully there is nothing wrong with my pull request, but feel free to let me know if you need any changes made.

I technically do also accept sending a faction_id in the request, however I already do an API request of the player on the backend and no other revive services there make use of a faction_id. So I opted to not add any new variable to include such a thing. I don't make use of any faction name/position string so I removed that from the request.

Fogest commented 3 months ago

If you need to do any testing with this and want to ensure you don't submit real revive requests. You can throw a #test into the app_info string anywhere. If app_info contains a "#test" somewhere in it, the request will get sent to a separate dev testing channel and won't go to revivers. Feel free to send as many test requests as needed if you need to run any tests.

I also will mention that if successful the response is an "201 - Resource Created", and it returns the Revive Request object as JSON:

{
    "data": {
        "created_at": "2024-06-22T14:33:32.000000Z",
        "updated_at": "2024-06-22T14:33:32.000000Z",
        "id": 1,
        "torn_player_id": 2254826,
        "torn_player_name": "Fogest",
        "torn_player_country": "Torn",
        "app_info": "#test REQUEST - NO REVIVE NEEDED",
        "faction_id": 17133
    }
}
Kwack-Kwack commented 3 months ago

I'm going to work so I haven't had a chance to test, but I just wanted to check that the endpoint has CORS headers (specifically Access-Control-Allow-Origin: *)?

The iOS port currently in beta requires these CORS headers or else safari will block the request.

Fogest commented 3 months ago

@Kwack-Kwack Yes it does :)

Sashank999 commented 3 months ago

Merge the latest master branch and also add a changelog entry. I will merge this.

Fogest commented 3 months ago

Merge the latest master branch and also add a changelog entry. I will merge this.

Pulled latest changes on master and added the changelog entry. I bumped the minor version, I wasn't sure if you'd want me to bump the minor or just bump the "build" version. Can swap it if needed 😄

Sashank999 commented 3 months ago

I should have been more specific. The top most entry in the current source code is the next release's changelog entry. You should add your change to any one category inside the entry. You don't need to change the date or the version as a script already does that during the packaging workflow this repo runs.

Fogest commented 3 months ago

@Sashank999 If you could take a look at pull request #801 . I missed the critical part of extensions needing permissions for each URL they make a request to. As such I also should have changed the manifest to include permissions for the new URL and removed the old one. This is currently causing errors for users as it indicates there are missing permissions.