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

Add poll #55

Merged
merged 64 commits into from
Sep 4, 2018
Merged

Add poll #55

merged 64 commits into from
Sep 4, 2018

Conversation

AlicePru
Copy link
Contributor

@AlicePru AlicePru commented Sep 3, 2018

Checklist

  • I've run mvn test from the root directory to see all new and existing tests pass
  • I've followed code style and run and ensured the code style is valid
  • I've created new tests if necessary.

Motivation and Context

#15
#16
#14
#41
#22
#23
#24
#50

Description

Were added opportunity to create poll and display feedback and voting in dependency of event status(open or closed) .
P.S. Not all methods covered by Unit Tests

MargoSank and others added 30 commits August 27, 2018 16:15
# Conflicts:
#	src/main/webapp/app/jsp/my-events.jsp
# Conflicts:
#	src/main/webapp/app/jsp/poll.jsp
# Conflicts:
#	src/main/java/lv/ctco/javaschool/eventorganaizer/boundary/EventOrganizationApi.java
#	src/main/webapp/app/jsp/poll.jsp
# Conflicts:
#	src/main/java/lv/ctco/javaschool/eventorganaizer/boundary/EventOrganizationApi.java
Fix function savePollToDB()
document.getElementById("edit").classList.remove("w3-hide");
document.getElementById("update").classList.remove("w3-hide");
document.getElementById("save").classList.add("w3-hide")
function checkFunction() {
Copy link
Contributor

Choose a reason for hiding this comment

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

checkFunction?

if(data.name == "" || data.name == " ") {
alert("Please input Event Name");
return;
function saveDataToDB() {
Copy link
Contributor

Choose a reason for hiding this comment

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

refactor function
validation should be in different function

document.getElementById("agenda").value = event.eventAgenda;
})
}
function getEventDataFromDB() {
Copy link
Contributor

Choose a reason for hiding this comment

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

is it form DB ?

var pair = vars[i].split("=");
if (pair[0] == variable) {
return pair[1];
function getQueryVariable(variable) {
Copy link
Contributor

Choose a reason for hiding this comment

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

variable names ?

Copy link
Contributor

Choose a reason for hiding this comment

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

check why it is used and remove if there is different way to get id.

if (event.eventStatus === "OPEN") {
document.getElementById("voting").classList.remove("w3-hide");
document.getElementById("feedback").classList.add("w3-hide");
getVotingFromDB();
Copy link
Contributor

Choose a reason for hiding this comment

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

DB ?

if (event.eventStatus === "CLOSED") {
document.getElementById("voting").classList.add("w3-hide");
document.getElementById("feedback").classList.remove("w3-hide");
getFeedbackFromDB()
Copy link
Contributor

Choose a reason for hiding this comment

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

DB ?

<div id="displayPoll">
</div>

<script>
Copy link
Contributor

Choose a reason for hiding this comment

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

scripts in different files

})
}

function getQueryVariable(variable) {
Copy link
Contributor

Choose a reason for hiding this comment

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

code dublicate

Copy link
Contributor

@AlexeyBuzdin AlexeyBuzdin left a comment

Choose a reason for hiding this comment

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

Please change

@Inject
private AnswersStore answersStore;


Copy link
Contributor

Choose a reason for hiding this comment

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

Please remove extra blank line

@Path("/{id}/getVotingPoll")
public List<PollDto> getVotingForEvent(@PathParam("id") Long id) {
List<Poll> poll = pollStore.getVotingPoll(id);
return poll.stream()
Copy link
Contributor

Choose a reason for hiding this comment

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

Please remove duplication by creating a mapPollToDto(poll) method

answerList.add(answer);
}
poll.setAnswers(answerList);
em.persist(poll);
Copy link
Contributor

Choose a reason for hiding this comment

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

Please put db logic into corresponding Store

@Path("/{id}/savePoll")
public void savePoll(PollDto pollDto, @PathParam("id") Long id) {
List<AnswerDto> answersString = pollDto.getAnswers();
//String[] answerList = answersString.split("\n");
Copy link
Contributor

Choose a reason for hiding this comment

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

Please remove comment if it is not needed

@Path("/{id}/savePoll")
public void savePoll(PollDto pollDto, @PathParam("id") Long id) {
List<AnswerDto> answersString = pollDto.getAnswers();
//String[] answerList = answersString.split("\n");
Copy link
Contributor

Choose a reason for hiding this comment

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

Do not commit comments please

@Inject
private AnswersStore answersStore;


Copy link
Contributor

Choose a reason for hiding this comment

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

Not needed extra line

@vadigerman vadigerman removed the request for review from avisnakovs September 4, 2018 11:17
vadigerman
vadigerman previously approved these changes Sep 4, 2018
vadigerman
vadigerman previously approved these changes Sep 4, 2018
AlexeyBuzdin
AlexeyBuzdin previously approved these changes Sep 4, 2018
…event-organizer into addPoll

# Conflicts:
#	src/main/webapp/app/jsp/add-event.jsp
#	src/main/webapp/app/jsp/event.jsp
#	src/main/webapp/app/jsp/my-events.jsp
#	src/main/webapp/app/jsp/start.jsp
#	src/main/webapp/index.jsp
#	src/main/webapp/style.css
@vadigerman vadigerman dismissed stale reviews from AlexeyBuzdin and themself via 847e713 September 4, 2018 13:02
@AlexeyBuzdin AlexeyBuzdin merged commit 90e5d6d into master Sep 4, 2018
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.

5 participants