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 accidental poll removal on status edit. #168

Merged
merged 5 commits into from
Mar 31, 2023

Conversation

bocops
Copy link
Collaborator

@bocops bocops commented Mar 20, 2023

As detected via #166, editing a status containing a poll using editStatus() will remove the poll. This clarifies our documentation by stating that:

  • poll statuses should not be edited using editStatus()
  • all poll data that should not be changed during an edit via editPoll() needs to be retrieved from the existing status first.

Removes default values from two poll parameters in editPoll() to avoid an accidental change when editing.

Fixes one unrelated typo in the actual request.

@bocops bocops requested a review from andregasser March 20, 2023 10:25
@codecov
Copy link

codecov bot commented Mar 20, 2023

Codecov Report

Merging #168 (0d71e7d) into master (c96b131) will decrease coverage by 0.03%.
The diff coverage is 65.38%.

Additional details and impacted files
@@             Coverage Diff              @@
##             master     #168      +/-   ##
============================================
- Coverage     44.97%   44.95%   -0.03%     
- Complexity      230      235       +5     
============================================
  Files            74       75       +1     
  Lines          2190     2169      -21     
  Branches        109      115       +6     
============================================
- Hits            985      975      -10     
+ Misses         1165     1148      -17     
- Partials         40       46       +6     
Impacted Files Coverage Δ
...c/main/kotlin/social/bigbone/rx/RxStatusMethods.kt 0.00% <0.00%> (ø)
.../kotlin/social/bigbone/api/method/StatusMethods.kt 72.43% <50.00%> (-2.97%) ⬇️
...otlin/social/bigbone/api/entity/ScheduledStatus.kt 80.55% <100.00%> (-4.23%) ⬇️
.../kotlin/social/bigbone/api/entity/data/PollData.kt 100.00% <100.00%> (ø)

Copy link
Owner

@andregasser andregasser left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The poll parameters should be refactored into a separat poll data class and the data class itself should then be placed in the *.entity.data package, as discussed in the associated issue.

bocops added 2 commits March 29, 2023 09:27
for use in Mastodon entities (ScheduledStatus) as well as in methods (post/edit/schedulePoll).
@bocops
Copy link
Collaborator Author

bocops commented Mar 29, 2023

I've now added the changes as discussed. Turns out, having two classes both named Poll is a bit problematic, though. My suggestion would be to rename the new one to PollData. @andregasser what do you think?

@andregasser
Copy link
Owner

Sure, that's fine! 👍🏻

@bocops
Copy link
Collaborator Author

bocops commented Mar 30, 2023

@andregasser Done.

@bocops bocops requested a review from andregasser March 30, 2023 07:08
Copy link
Owner

@andregasser andregasser left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Approved!

@andregasser andregasser merged commit a2d42a2 into andregasser:master Mar 31, 2023
@bocops bocops deleted the fix_poll_removal_on_edit branch April 6, 2023 09:13
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.

2 participants