Add election result validator #1548

BelghmiAmine · 2023-05-05T10:24:50Z

This pull request is about the decentralized communication part.

The new server to server architecture implies servers receiving ResultElection messages, which previously were never received by a server. This pull request implements and tests the ResultElection Validator.

K1li4nL

Just a few things that should lighten the code a bit ;)

be2-scala/src/main/scala/ch/epfl/pop/pubsub/graph/validators/ElectionValidator.scala

K1li4nL

Just a few things that should lighten the code a bit ;)

Ajkunas

Only some nitpicks, othw all good

be2-scala/src/main/scala/ch/epfl/pop/pubsub/graph/validators/ElectionValidator.scala

be2-scala/src/test/scala/util/examples/JsonRpcRequestExample.scala

be2-scala/src/main/scala/ch/epfl/pop/pubsub/graph/validators/ElectionValidator.scala

Ajkunas

There is still some changes to do. Do not hesitate to ask me questions if some of my comments are not clear :))
Also, I was wondering, should we also check if our ballot options in the ResultElection correspond to the ballot options in our SetupElection?
We can have the case, where all ids are checked and everything passed, but we do not have any ballot that are coherent. What do you think?

be2-scala/src/main/scala/ch/epfl/pop/pubsub/graph/validators/ElectionValidator.scala

be2-scala/src/test/scala/ch/epfl/pop/pubsub/graph/validators/ElectionValidatorSuite.scala

be2-scala/src/main/scala/ch/epfl/pop/pubsub/graph/validators/ElectionValidator.scala

Ajkunas

There is some changes to do still. Do not hesitate to ask me questions if some of my comments are not clear :))
Also, I was wondering, should we also check if our ballot options in the ResultElection correspond to the ballot options in our SetupElection?
We can have the case, where all ids are checked and everything passed, but we do not have any ballot that are coherent. What do you think?

BelghmiAmine · 2023-05-25T09:05:06Z

@Ajkunas indeed, since ballot options are not used in the question id's computation, we can add a check for ballot options consistency.

Ajkunas

Some nitpicks and some comments. ;)
You have a lot of already defined functions in Scala, try to take a look at the documentation when you want to implement smth, your code could be more concise.
Othw, good job :)

be2-scala/src/main/scala/ch/epfl/pop/pubsub/graph/validators/ElectionValidator.scala

be2-scala/src/test/scala/ch/epfl/pop/pubsub/graph/validators/ElectionValidatorSuite.scala

be2-scala/src/test/scala/util/examples/JsonRpcRequestExample.scala

be2-scala/src/main/scala/ch/epfl/pop/pubsub/graph/validators/ElectionValidator.scala

Ajkunas

Still some nitpicks to correct. After taking in consideration my comments, I will be happy to approve the PR.

be2-scala/src/main/scala/ch/epfl/pop/pubsub/graph/validators/ElectionValidator.scala

be2-scala/src/test/scala/ch/epfl/pop/pubsub/graph/validators/ElectionValidatorSuite.scala

Ajkunas · 2023-05-29T09:48:29Z

be2-scala/src/test/scala/util/examples/JsonRpcRequestExample.scala

@@ -10,6 +11,8 @@ import util.examples.Election.OpenElectionExamples._
 import util.examples.Election.SetupElectionExamples.{ELECTION_ID, _}
 import util.examples.Election.EndElectionExamples._
 import util.examples.Election.KeyElectionExamples._
+import util.examples.Election.ResultElectionExamples._
+import util.examples.Election.{ResultElectionExamples, SetupElectionExamples}


You could remove this line, since it is a repetition of previous lines (line 14 and line 11).

Okay, but now why do you include SetupElectionExamples ? You already import it in line 10.

Ajkunas

Last nitpicks :)

Ajkunas · 2023-05-29T12:11:02Z

be2-scala/src/test/scala/util/examples/JsonRpcRequestExample.scala

@@ -10,6 +11,8 @@ import util.examples.Election.OpenElectionExamples._
 import util.examples.Election.SetupElectionExamples.{ELECTION_ID, _}
 import util.examples.Election.EndElectionExamples._
 import util.examples.Election.KeyElectionExamples._
+import util.examples.Election.ResultElectionExamples._
+import util.examples.Election.{ResultElectionExamples, SetupElectionExamples}


Okay, but now why do you include SetupElectionExamples ? You already import it in line 10.

be2-scala/src/test/scala/ch/epfl/pop/pubsub/graph/validators/ElectionValidatorSuite.scala

sonarcloud · 2023-05-30T19:53:16Z

[PoP - Be2-Scala] Kudos, SonarCloud Quality Gate passed!

0 Bugs
0 Vulnerabilities
0 Security Hotspots
1 Code Smell

89.6% Coverage
0.0% Duplication

sonarcloud · 2023-05-30T19:53:32Z

[PoP - Be1-Go] Kudos, SonarCloud Quality Gate passed!

0 Bugs
0 Vulnerabilities
0 Security Hotspots
0 Code Smells

No Coverage information
No Duplication information

sonarcloud · 2023-05-30T19:56:49Z

[PoP - Fe2-Android] Kudos, SonarCloud Quality Gate passed!

0 Bugs
0 Vulnerabilities
0 Security Hotspots
0 Code Smells

No Coverage information
No Duplication information

sonarcloud · 2023-05-30T19:59:56Z

[PoP - Fe1-Web] Kudos, SonarCloud Quality Gate passed!

0 Bugs
0 Vulnerabilities
0 Security Hotspots
0 Code Smells

No Coverage information
No Duplication information

Ajkunas

LGTM

BelghmiAmine requested a review from a team as a code owner May 5, 2023 10:24

BelghmiAmine changed the title ~~WIP : Add election result validator~~ Add election result validator May 8, 2023

K1li4nL added the be2-scala label May 8, 2023

K1li4nL requested changes May 8, 2023

View reviewed changes

Ajkunas suggested changes May 9, 2023

View reviewed changes

BelghmiAmine added 2 commits May 14, 2023 13:34

add electioResultValidator

9fef9ca

add electioResultValidator tests.

d448214

BelghmiAmine force-pushed the work_be2_amine_add_electionResultValidator branch from da327d9 to d448214 Compare May 14, 2023 11:36

BelghmiAmine added 3 commits May 15, 2023 11:18

typo fix

f0a4be8

add test

3aa1ac5

bug fix for testing election result ids.

7ad781c

K1li4nL requested changes May 18, 2023

View reviewed changes

modify validation of result election ids method.

5ffa629

Ajkunas suggested changes May 24, 2023

View reviewed changes

resolved nitpick and added check for ballot options.

4630da6

Ajkunas suggested changes May 27, 2023

View reviewed changes

Ajkunas reviewed May 27, 2023

View reviewed changes

be2-scala/src/main/scala/ch/epfl/pop/pubsub/graph/validators/ElectionValidator.scala Outdated Show resolved Hide resolved

resolved nitpicks.

27cf56c

Ajkunas suggested changes May 29, 2023

View reviewed changes

resolved nitpicks.

7fa165a

Ajkunas suggested changes May 29, 2023

View reviewed changes

BelghmiAmine added 2 commits May 29, 2023 17:45

corrected last nitpicks.

d78616c

import fixing.

23d9506

Ajkunas approved these changes May 31, 2023

View reviewed changes

K1li4nL approved these changes May 31, 2023

View reviewed changes

BelghmiAmine merged commit 95c26b5 into master May 31, 2023

BelghmiAmine deleted the work_be2_amine_add_electionResultValidator branch May 31, 2023 09:44

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Add election result validator #1548

Add election result validator #1548

BelghmiAmine commented May 5, 2023

K1li4nL left a comment

K1li4nL left a comment

Ajkunas left a comment

Ajkunas left a comment •

edited

Loading

Ajkunas left a comment

BelghmiAmine commented May 25, 2023

Ajkunas left a comment

Ajkunas left a comment

Ajkunas May 29, 2023

Ajkunas May 29, 2023

Ajkunas left a comment

Ajkunas May 29, 2023

sonarcloud bot commented May 30, 2023

sonarcloud bot commented May 30, 2023

sonarcloud bot commented May 30, 2023

sonarcloud bot commented May 30, 2023

Ajkunas left a comment

Add election result validator #1548

Add election result validator #1548

Conversation

BelghmiAmine commented May 5, 2023

K1li4nL left a comment

Choose a reason for hiding this comment

K1li4nL left a comment

Choose a reason for hiding this comment

Ajkunas left a comment

Choose a reason for hiding this comment

Ajkunas left a comment • edited Loading

Choose a reason for hiding this comment

Ajkunas left a comment

Choose a reason for hiding this comment

BelghmiAmine commented May 25, 2023

Ajkunas left a comment

Choose a reason for hiding this comment

Ajkunas left a comment

Choose a reason for hiding this comment

Ajkunas May 29, 2023

Choose a reason for hiding this comment

Ajkunas May 29, 2023

Choose a reason for hiding this comment

Ajkunas left a comment

Choose a reason for hiding this comment

Ajkunas May 29, 2023

Choose a reason for hiding this comment

sonarcloud bot commented May 30, 2023

sonarcloud bot commented May 30, 2023

sonarcloud bot commented May 30, 2023

sonarcloud bot commented May 30, 2023

Ajkunas left a comment

Choose a reason for hiding this comment

Ajkunas left a comment •

edited

Loading