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

feat: Order feedback by date #2261

Merged
merged 16 commits into from
Jun 27, 2019
Merged

feat: Order feedback by date #2261

Changes from 1 commit
Commits
Show all changes
16 commits
Select commit Hold shift + click to select a range
278d169
feat: Order feedback by date
atm1504 Jun 12, 2019
488e7c8
Merge branch 'development' into atm_order_feedback
atm1504 Jun 12, 2019
e0a63db
Merge branch 'development' into atm_order_feedback
atm1504 Jun 14, 2019
13416e7
Trigger circle ci
atm1504 Jun 15, 2019
f9f623f
Merge branch 'development' into atm_order_feedback
atm1504 Jun 15, 2019
7b12dd2
Merge branch 'development' into atm_order_feedback
atm1504 Jun 19, 2019
8c7497e
Merge branch 'development' into atm_order_feedback
atm1504 Jun 22, 2019
c20af0c
Merge branch 'development' into atm_order_feedback
atm1504 Jun 23, 2019
5c0a5bc
removed duplication
atm1504 Jun 23, 2019
c8b162d
fix: remove object creation
atm1504 Jun 23, 2019
5973d84
change use of stringset in prefmanager
atm1504 Jun 24, 2019
5b17ba1
make private val, stringSet in prefManager
atm1504 Jun 25, 2019
3f38282
Merge branch 'development' into atm_order_feedback
atm1504 Jun 25, 2019
b0d5754
replace var with val
atm1504 Jun 25, 2019
81db93a
Merge branch 'development' into atm_order_feedback
atm1504 Jun 25, 2019
50a91f1
Merge branch 'development' into atm_order_feedback
atm1504 Jun 27, 2019
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
26 changes: 12 additions & 14 deletions ...ava/org/fossasia/susi/ai/skills/skilldetails/adapters/recycleradapters/FeedbackAdapter.kt
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -54,31 +54,29 @@ class FeedbackAdapter(
arrangedFeedbackList.clear()
feedbackResponse.feedbackList.forEach { item ->
if (item.email == PrefManager.getStringSet(Constant.SAVED_EMAIL)?.iterator()?.next()) {
Copy link
Member

Choose a reason for hiding this comment

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

PrefManager.getStringSet(Constant.SAVED_EMAIL)?.iterator()?.next() what?

Copy link
Member Author

Choose a reason for hiding this comment

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

PrefManager.getStringSet(Constant.SAVED_EMAIL)?.iterator()?.next() what?

This method has been used throughout the app while extracting email from the PrefManager. So, I used it else I would have simply used PrefManager.getString(Constant.SAVED_EMAIL,"")

Copy link
Member

Choose a reason for hiding this comment

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

Use yours

Copy link
Member Author

@atm1504 atm1504 Jun 25, 2019 •
edited
Loading

Choose a reason for hiding this comment

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

@iamareebjamal the app is crashing if I use the getString method in prefmanger to extract the email, but it is working if I use getStringSet. Probably that's the reason getStringSet has been used throughout the app.

Copy link
Member

Choose a reason for hiding this comment

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

OK, revert but it is very stupid

Copy link
Member Author

Choose a reason for hiding this comment

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

Already reverted in the latest update.

var feedback = Feedback()
feedback.email = item.email
feedback.userName = item.userName
feedback.avatar = item.avatar
feedback.timestamp = item.timestamp
feedback.feedback = item.feedback
arrangedFeedbackList.add(feedback)
addFeedback(item)
}
}

var reverseResponse = feedbackResponse.feedbackList.asReversed()
reverseResponse.forEach { item ->
if (item.email != PrefManager.getStringSet(Constant.SAVED_EMAIL)?.iterator()?.next()) {
var feedback = Feedback()
feedback.email = item.email
feedback.userName = item.userName
feedback.avatar = item.avatar
feedback.timestamp = item.timestamp
feedback.feedback = item.feedback
arrangedFeedbackList.add(feedback)
addFeedback(item)
}
}
Timber.d("Arranged the feedback")
}

private fun addFeedback(item: Feedback) {
var feedback = Feedback()
feedback.email = item.email
feedback.userName = item.userName
feedback.avatar = item.avatar
feedback.timestamp = item.timestamp
feedback.feedback = item.feedback
Copy link
Member

Choose a reason for hiding this comment

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

Why are you copying it?

Copy link
Member Author

Choose a reason for hiding this comment

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

Currently what we have is that we get a response of the feedback in the order of newest at the top. But what we want is that, if the user logged in has given any feedback, then it should come at the top irrespective of the time he gave that feedback. After user feedback is shown, we need to show the rest of the feedbacks in the order with the newest at the top. So, to arrange the list in such an order, I am keeping a separate list of those items and showing these data, with actually affecting the server response directly.

Copy link
Member

Choose a reason for hiding this comment

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

Why are you creating a new feedback object and copying the same data in it?

Copy link
Member Author

Choose a reason for hiding this comment

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

The actual feedback response has many different kinds of data, we don't need those data to display or even to iterate any operation. So, I made a feedback object, which can copy the response that we need, instead of copying all the data.

Copy link
Member

Choose a reason for hiding this comment

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

If you don't create a new object, will there be any problem?

Copy link
Member Author

Choose a reason for hiding this comment

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

No, what I can do is that I can create the object only once and reuse it every time.

Copy link
Member

@iamareebjamal iamareebjamal Jun 23, 2019 •
edited
Loading

Choose a reason for hiding this comment

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

Why do you even want to create an object?

Copy link
Member Author

Choose a reason for hiding this comment

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

We don't need. I understood what u wanted to say. Updating the pr accordingly.

arrangedFeedbackList.add(feedback)
}

@NonNull
override fun onBindViewHolder(holder: FeedbackViewHolder, position: Int) {
if (arrangedFeedbackList.isNotEmpty()) {
Expand Down