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.

Sign up for GitHub

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

Jump to bottom

Fix Error on reveal voting subject #314

Merged
merged 1 commit into from
May 26, 2019
Merged

Fix Error on reveal voting subject #314

merged 1 commit into from
May 26, 2019

Conversation

1-alex98
Copy link
Member

@1-alex98 1-alex98 commented May 8, 2019

Fixes #313

@1-alex98 1-alex98 added the ready label May 8, 2019
@1-alex98 1-alex98 force-pushed the bugfix/#313-error-reveal-vote branch 3 times, most recently from b9f2567 to adf5c7f Compare May 8, 2019 00:32
Brutus5000
Brutus5000 requested changes May 8, 2019
View reviewed changes
Copy link
Member

@Brutus5000 Brutus5000 left a comment •
edited
Loading

Choose a reason for hiding this comment

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

Outside of the review: To be honest, I have no clue what is happening there. Despite using streams this is unreadable.

src/main/java/com/faforever/api/data/listeners/VotingSubjectEnricher.java Outdated
int newAlternativeOrdinal = votingAnswer.getAlternativeOrdinal() + 1;
votingAnswer.getVote().getVotingAnswers().stream()
.filter(votingAnswerFilter -> votingAnswerFilter.getVotingChoice() != null
&& votingAnswerFilter.getVotingChoice().getVotingQuestion().equals(votingAnswer.getVotingChoice().getVotingQuestion())
Copy link
Member

Choose a reason for hiding this comment

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

Use Objects.equals to prevent another NPE

Copy link
Member Author

Choose a reason for hiding this comment

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

They should just not use the freaked out "alternative voting"

@1-alex98 1-alex98 force-pushed the bugfix/#313-error-reveal-vote branch from adf5c7f to 77921ae Compare May 16, 2019 21:52
@1-alex98 1-alex98 requested a review from Brutus5000 May 16, 2019 21:54
@1-alex98 1-alex98 force-pushed the bugfix/#313-error-reveal-vote branch from 77921ae to bb443f9 Compare May 26, 2019 19:52
@Brutus5000 Brutus5000 merged commit 3616b46 into develop May 26, 2019
@Brutus5000 Brutus5000 deleted the bugfix/#313-error-reveal-vote branch September 5, 2019 19:51
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@Brutus5000 Brutus5000 Brutus5000 approved these changes

@micheljung micheljung Awaiting requested review from micheljung

@Geosearchef Geosearchef Awaiting requested review from Geosearchef

Assignees
No one assigned
Labels
ready
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.

Error on reveal alternative vote
2 participants
@1-alex98 @Brutus5000