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

Fix #196: Fix incompatibilities with the Mastodon API v2.0.0. #197

Merged
merged 8 commits into from
Nov 29, 2017

Conversation

n1k0
Copy link
Owner

@n1k0 n1k0 commented Oct 22, 2017

This is an attempt at fixing breaking changes with the Mastodon API v2.0.0 (#196), notably those where the unique ids are now strings instead of ints.

The fix ensures previous Mastodon server versions are still working properly.

Users are invited to test this patch locally and give their feedback on this issue if anything is broken or missing.

@n1k0 n1k0 requested a review from vjousse October 22, 2017 09:51
@n1k0
Copy link
Owner Author

n1k0 commented Oct 22, 2017

Ideally we should abstract all the id fields so they have their own type, eg.

type StatusId = StatusId String

Reasoning being, as per rtfeldman's example SPA:

Identifiers such as CommentId, Username, and Slug - which are used to uniquely identify comments, users, and articles, respectively - are implemented as union types. If we used e.g. type alias Username = String, we could mistakenly pass a Username to an API call expecting a Slug, and it would still compile. We can rule bugs like that out by implementing identifiers as union types.

That's a bunch of code to update and I'm kinda lazy these days, so any help doing so would be greatly appreciated.

@n1k0 n1k0 mentioned this pull request Oct 22, 2017
@DirtyF
Copy link
Collaborator

DirtyF commented Oct 22, 2017

That's a bunch of code to update and I'm kinda lazy these days, so any help doing so would be greatly appreciated.

n1k0 seems like a good candidate for hacktoberfest 🍺

@n1k0
Copy link
Owner Author

n1k0 commented Oct 22, 2017

Ah, I know nothing about it and just spent all my available amount of time dedicated to do something other than enjoying my weekend, would you mind taking care of that? :)

@n1k0
Copy link
Owner Author

n1k0 commented Oct 22, 2017

If anyone is interested into contributing, I've migrated the Status record type to use a specific type for status ids in 418bac1.

We should do the same for Account, Media and every other entity having a string id.

@bnjbvr
Copy link

bnjbvr commented Oct 22, 2017

Thanks for giving it a look so quickly, and sorry to undermine your weekend! When I try this PR, npm install and package and npm start, I run into the following error when accessing /:

Trying to initialize the `Main` module with an unexpected flag.
I tried to convert it to an Elm value, but ran into this problem:

Expecting a String at _.clients but instead got: null

Is there something I am doing wrong?

@n1k0
Copy link
Owner Author

n1k0 commented Oct 22, 2017

@bnjbvr Can you please update and check again? Thanks!

@bnjbvr
Copy link

bnjbvr commented Oct 28, 2017

Works like a charm, thank you so much!

@n1k0
Copy link
Owner Author

n1k0 commented Nov 27, 2017

Things should be working entirely now, I've deployed this branch to production at https://n1k0.github.io/tooty/

Feedback would be much appreciated!

@bnjbvr
Copy link

bnjbvr commented Nov 27, 2017

Works like a charm, thank you very much!

@chantoine
Copy link

chantoine commented Nov 27, 2017 via email

@n1k0 n1k0 merged commit 2e89ad0 into master Nov 29, 2017
@n1k0 n1k0 deleted the fix-bc-break-2.0 branch November 29, 2017 13:06
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants