feat: Order feedback by date #2261
Conversation
|
@arundhati24 @batbrain7 @iamareebjamal please review it. |
|
@liveHarshit can u please review it. So that @iamareebjamal can finalize it easily. |
| feedback.avatar = item.avatar | ||
| feedback.timestamp = item.timestamp | ||
| feedback.feedback = item.feedback | ||
| arrangedFeedbackList.add(feedback) |
|
@liveHarshit please review now. : ) |
| feedback.userName = item.userName | ||
| feedback.avatar = item.avatar | ||
| feedback.timestamp = item.timestamp | ||
| feedback.feedback = item.feedback |
There was a problem hiding this comment.
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.
There was a problem hiding this comment.
Why are you creating a new feedback object and copying the same data in it?
There was a problem hiding this comment.
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.
There was a problem hiding this comment.
If you don't create a new object, will there be any problem?
There was a problem hiding this comment.
No, what I can do is that I can create the object only once and reuse it every time.
There was a problem hiding this comment.
Why do you even want to create an object?
There was a problem hiding this comment.
We don't need. I understood what u wanted to say. Updating the pr accordingly.
| private fun arrangeFeedbacks() { | ||
| arrangedFeedbackList.clear() | ||
| feedbackResponse.feedbackList.forEach { item -> | ||
| if (item.email == PrefManager.getStringSet(Constant.SAVED_EMAIL)?.iterator()?.next()) { |
There was a problem hiding this comment.
PrefManager.getStringSet(Constant.SAVED_EMAIL)?.iterator()?.next() what?
There was a problem hiding this comment.
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,"")
There was a problem hiding this comment.
@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.
There was a problem hiding this comment.
OK, revert but it is very stupid
There was a problem hiding this comment.
Already reverted in the latest update.
|
@iamareebjamal please review it now. |
| @@ -26,6 +30,7 @@ class FeedbackAdapter( | |||
| RecyclerView.Adapter<FeedbackViewHolder>(), FeedbackViewHolder.ClickListener { | |||
|
|
|||
| private val clickListener: FeedbackViewHolder.ClickListener = this | |||
| var arrangedFeedbackList: ArrayList<Feedback> = ArrayList() | |||
There was a problem hiding this comment.
Where are you reassigning the variable? From which external class are you accessing the variable?
Seems I have to repeat the same review again and again. I will not review again unless you learn from past reviews and stop making same mistakes again and again
There was a problem hiding this comment.
Where are you reassigning the variable? From which external class are you accessing the variable?
Seems I have to repeat the same review again and again. I will not review again unless you learn from past reviews and stop making same mistakes again and again
Sorry for the inconvenience caused. I will definitely take care of now and not give any other chance for such mistakes.
I have updated it. Also, I have changed it to getStringSet, else the app is crashing. During the successful login, the email id is saved using StringSet. So we have to use stringSet to retrieve the data.
| } | ||
| } | ||
|
|
||
| var reverseResponse = feedbackResponse.feedbackList.asReversed() |
There was a problem hiding this comment.
Where are you reassigning this? I told you last time to check before submitting PRs for review
|
@iamareebjamal please review it. I have changed all var to val wherever possible. Also, I went through the code once again to look for such mistakes. |
|
@iamareebjamal please review it now. |
|
@iamareebjamal please review it. |
Fixes #2023 #1453
Changes:
The feedback is now ordered in a way such that, if the user has feedback, then it gets displayed at the top and the rests are ordered by the newest first.
Screenshots for the change:
Apk:
app-fdroid-debug.apk.zip