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

[bugfix] fix unordered favorites #1245

Merged

Conversation

voigt
Copy link
Contributor

@voigt voigt commented Dec 9, 2022

Description

This pull request fixes the ordering of favorites list.

closes #1151 (improves solution of #1236)

Checklist

Please put an x inside each checkbox to indicate that you've read and followed it: [ ] -> [x]

If this is a documentation change, only the first checkbox must be filled (you can delete the others if you want).

  • I/we have read the GoToSocial contribution guidelines.
  • I/we have discussed the proposed changes already, either in an issue on the repository, or in the Matrix chat.
  • I/we have performed a self-review of added code.
  • I/we have written code that is legible and maintainable by others.
  • I/we have commented the added code, particularly in hard-to-understand areas.
  • I/we have made any necessary changes to documentation.
  • I/we have added tests that cover new code.
  • I/we have run tests and they pass locally with the changes.
  • I/we have run go fmt ./... and golangci-lint run.

@voigt voigt force-pushed the fix/1151_-_unordered_favorites branch from 55357b8 to 3c67506 Compare December 9, 2022 22:15
internal/db/bundb/timeline.go Outdated Show resolved Hide resolved
@voigt voigt force-pushed the fix/1151_-_unordered_favorites branch from 3c67506 to a202bd4 Compare December 9, 2022 22:22
@voigt
Copy link
Contributor Author

voigt commented Dec 9, 2022

I'm still unsure whether to provide a test. What exactly would you like to be seen tested in this case?

@tsmethurst
Copy link
Contributor

I'm still unsure whether to provide a test. What exactly would you like to be seen tested in this case?

Will respond to this later today, I've got some ideas :)

@voigt
Copy link
Contributor Author

voigt commented Dec 11, 2022

@tsmethurst Sure, let me know - happy to implement and improve testing! :)

@tsmethurst
Copy link
Contributor

So I think a good way of more or less end-to-end testing this would be to just put some faves in the db, then retrieve them via the handler (so, a test in here somewhere: https://github.com/superseriousbusiness/gotosocial/tree/main/internal/api/client/favourites), and check that they're ordered correctly, and that the link headers are what we would expect. You can look around in some of the other api packages for some inspiration on testing, since we do something similar in a lot of places :) What do you think?

@voigt voigt force-pushed the fix/1151_-_unordered_favorites branch from a202bd4 to 0e76843 Compare December 12, 2022 19:47
@voigt voigt force-pushed the fix/1151_-_unordered_favorites branch from 0e76843 to 5222bfc Compare December 12, 2022 21:34
@voigt
Copy link
Contributor Author

voigt commented Dec 12, 2022

@tsmethurst I added a test, I hope this is what you expect. I added some test data, as you suggested, fetching the favorites of local_account_1 and checking whether the first and last items are correct.

@tsmethurst
Copy link
Contributor

Great stuff, thank you! Can you add the license text header to the new files (just copy and paste from another .go file)? Then this is ready to be merged imo 👍

@voigt
Copy link
Contributor Author

voigt commented Dec 13, 2022

✅ Done!

@tsmethurst tsmethurst merged commit 8703933 into superseriousbusiness:main Dec 13, 2022
@tsmethurst
Copy link
Contributor

Thanks!

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.

[bug] unordered Favorites
3 participants