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

feat: add event feedback #1601

Merged
merged 2 commits into from Apr 18, 2019
Merged

feat: add event feedback #1601

merged 2 commits into from Apr 18, 2019

Conversation

aggarwalpulkit596
Copy link
Contributor

@aggarwalpulkit596 aggarwalpulkit596 commented Apr 13, 2019 •
edited

Fixes #1591 part 1

Changes: added event feedback section for a particular event

Screenshots for the change:
screenshot-1555230073908

@aggarwalpulkit596 aggarwalpulkit596 changed the title [WIP]feat: add event feedback feat: add event feedback Apr 13, 2019
@auto-label auto-label bot added the feature label Apr 13, 2019
@aggarwalpulkit596
Copy link
Contributor Author

@liveHarshit @iamareebjamal review

nikit19
nikit19 reviewed Apr 13, 2019
View reviewed changes
Copy link
Member

@nikit19 nikit19 left a comment

Choose a reason for hiding this comment

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

Screenshot pls

@aggarwalpulkit596
Copy link
Contributor Author

@nikit19 couldn't add a screenshot as there was no Event with a feedback and i was unable to create a feedback manually using the api.I already have created an issue on server for the same

@aggarwalpulkit596
Copy link
Contributor Author

@nikit19 updated

@aggarwalpulkit596
Copy link
Contributor Author

@liveHarshit please review

liveHarshit
liveHarshit reviewed Apr 15, 2019
View reviewed changes
Copy link
Member

@liveHarshit liveHarshit left a comment

Choose a reason for hiding this comment

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

Reset lgtm

app/src/main/java/org/fossasia/openevent/general/event/EventDetailsViewModel.kt Outdated Show resolved Hide resolved
@aggarwalpulkit596
Copy link
Contributor Author

@liveHarshit @iamareebjamal updated

@aggarwalpulkit596
Copy link
Contributor Author

@iamareebjamal anything left out?

@iamareebjamal
Copy link
Member

PRF

addiegupta
addiegupta reviewed Apr 16, 2019
View reviewed changes
app/src/main/java/org/fossasia/openevent/general/event/EventDetailsFragment.kt Outdated
eventViewModel.eventFeedback.observe(viewLifecycleOwner, Observer {
feedbackAdapter.addAll(it)
if (it.isEmpty()) {
rootView.feedbackContainer.visibility = View.GONE
Copy link
Contributor

Choose a reason for hiding this comment

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

Replace with rootView.feedbackContainer.isVisible = false

Copy link
Member

Choose a reason for hiding this comment

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

Replace with rootView.feedbackContainer.isVisible = false

What is the advantage?

Copy link
Member

Choose a reason for hiding this comment

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

Much clearer and using booleans (primitives)

Copy link
Contributor Author

Choose a reason for hiding this comment

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

@liveHarshit any other change ?

@aggarwalpulkit596
Copy link
Contributor Author

@liveHarshit updated

liveHarshit
liveHarshit previously approved these changes Apr 17, 2019
View reviewed changes
iamareebjamal
iamareebjamal reviewed Apr 17, 2019
View reviewed changes
app/src/main/java/org/fossasia/openevent/general/event/feedback/Feedback.kt Outdated
val id: Long,
val rating: String?,
val comment: String?,
@JsonProperty("deleted-at")
Copy link
Member

Choose a reason for hiding this comment

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

Unneeded

Copy link
Contributor Author

Choose a reason for hiding this comment

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

@iamareebjamal updated

@iamareebjamal iamareebjamal merged commit 32d3792 into fossasia:development Apr 18, 2019
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@nikit19 nikit19 nikit19 left review comments

@addiegupta addiegupta addiegupta left review comments

@iamareebjamal iamareebjamal iamareebjamal approved these changes

@liveHarshit liveHarshit liveHarshit left review comments

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

Successfully merging this pull request may close these issues.

Add Event Feedbacks
5 participants
@aggarwalpulkit596 @iamareebjamal @nikit19 @addiegupta @liveHarshit