Skip to content
This repository was archived by the owner on Oct 15, 2025. It is now read-only.

number of issued ballots are recorded #194 #195#263

Merged
magwas merged 10 commits intodevelopfrom
feature/number_of_issued_ballots_are_recorded_for_admin_194
Mar 17, 2019
Merged

number of issued ballots are recorded #194 #195#263
magwas merged 10 commits intodevelopfrom
feature/number_of_issued_ballots_are_recorded_for_admin_194

Conversation

@valentinbujdoso
Copy link
Copy Markdown
Contributor

@valentinbujdoso valentinbujdoso commented Mar 16, 2019

I hope it covers your expectations.
#194
#195

@valentinbujdoso valentinbujdoso requested a review from magwas March 16, 2019 11:21
@magwas
Copy link
Copy Markdown
Member

magwas commented Mar 16, 2019

conflicts

@valentinbujdoso
Copy link
Copy Markdown
Contributor Author

Fixed

Bujdoso Valentin added 2 commits March 16, 2019 15:21
@valentinbujdoso
Copy link
Copy Markdown
Contributor Author

@magwas Could you fix the sonarcloud to not whine about public properties of an Entity here too?

@valentinbujdoso
Copy link
Copy Markdown
Contributor Author

Sorry I would not like to be rude. :(
So I just would like to ask you take a look about sonarcloud public property check. :)

@magwas
Copy link
Copy Markdown
Member

magwas commented Mar 16, 2019

It is not being rude, Sonar is sometimes whining because does not understand the situation.

Comment thread src/test/java/org/rulez/demokracia/pdengine/ObtainBallotTest.java Outdated
Comment thread src/test/java/org/rulez/demokracia/pdengine/ObtainBallotTest.java Outdated
@valentinbujdoso valentinbujdoso changed the title number of issued ballots are recorded for admin #194 number of issued ballots are recorded #194 #195 Mar 16, 2019
@valentinbujdoso valentinbujdoso requested a review from magwas March 16, 2019 21:25
Comment thread src/test/java/org/rulez/demokracia/pdengine/ObtainBallotTest.java Outdated
Vote vote = getVote(id);

if(adminKey.equals("anon")) {
if(adminKey.equals(vote.adminKey))
Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

Well, the requirement was confusing. You can choose to leave the implementation as is, as this is not your mistake, and you are not obliged to read minds.
For the record I try to reword the intent of the two behaviours:

  • adminkey can be either "user" or the adminkey of the vote
  • if adminkey is "user", and the user have all the needed assurances, then the ballot is registered for the WS user, and at most one ballot can be issued for that user.
  • if the adminkey is the vote's adminkey, then the ballot is registered to "admin"

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

ok I will take a look later. :)

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

So I pushed my changes and I observed that. You use user instead of anon. I mean I changed the anon checks but it is on the beharviors too.

For example:
@tested_feature("Manage votes")
@tested_operation("Obtain ballot")
@tested_behaviour("if adminKey is anon, the user should have all the neededAssurances")

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

Btw it was a very good specificaton thanks :)

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

Is there any future plan to explain more detailed the tasks/specification?

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

Of course I have to update the architecture model. See #267

@magwas
Copy link
Copy Markdown
Member

magwas commented Mar 17, 2019

testcasenumber=2

@magwas magwas merged commit ddae53e into develop Mar 17, 2019
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants