feat: Show similar events based on event topic #198

dreadpool2 · 2018-06-24T21:45:31Z

Fixes #123

iamareebjamal · 2018-06-24T22:45:03Z

app/src/main/java/org/fossasia/openevent/general/similar/EventTopicId.kt

@@ -0,0 +1,11 @@
+package org.fossasia.openevent.general.similar


What similar?

iamareebjamal · 2018-06-24T22:45:43Z

app/src/main/java/org/fossasia/openevent/general/similar/EventTopicsDao.kt

+    fun getEventTopicOfEvent(eventId: Long): Flowable<EventTopic>
+*/
+    @Query("UPDATE Event SET favorite = :favorite WHERE id = :eventId")
+    fun setFavorite(eventId: Long, favorite: Boolean)


Ummm This is EventDao

iamareebjamal · 2018-06-24T22:46:21Z

app/src/main/java/org/fossasia/openevent/general/similar/SimilarEventsViewModel.kt

+    val similarEvents = MutableLiveData<List<Event>>()
+    val error = MutableLiveData<String>()
+
+    fun getSimilarEvents(id: Long) {


A void getter!!!

Will change the function name 😄

Updated PR 👍

simarsingh24 · 2018-06-25T05:06:46Z

app/src/main/java/org/fossasia/openevent/general/event/EventsRecyclerAdapter.kt

-        val view = LayoutInflater.from(parent.context).inflate(R.layout.item_card_events, parent, false)
-        return EventViewHolder(view)
+        val eventView: View
+        if (eventLayout.equals("similarEvents")) {


Keep similarEvents and events in some constant string and use reference here

simarsingh24 · 2018-06-25T05:08:36Z

app/src/main/java/org/fossasia/openevent/general/similarEvents/EventTopicApi.kt

    @GET("event-topics/{id}")
    fun getEventTopic(@Path("id") id: Long): Single<EventTopic>
-
+*/


why are you keeping commented code?

simarsingh24 · 2018-06-25T05:09:41Z

app/src/main/java/org/fossasia/openevent/general/similarEvents/EventTopicService.kt

+                        .map {
+                            eventTopicsDao.insertSimilarEvents(it)
+                        }
+                        .toFlowable()


take this to the line above

simarsingh24 · 2018-06-25T05:09:52Z

app/src/main/java/org/fossasia/openevent/general/similarEvents/EventTopicService.kt

+                            eventTopicsDao.insertEventTopic(it)
+                            it
+                        }
+                        .toFlowable()


simarsingh24 · 2018-06-25T05:10:11Z

app/src/main/java/org/fossasia/openevent/general/similarEvents/EventTopicsDao.kt

+/*
+    @Insert(onConflict = OnConflictStrategy.REPLACE)
+    fun insertEventTopic(eventTopic: EventTopic)
+*/


commented code?

simarsingh24 · 2018-06-25T05:10:21Z

app/src/main/java/org/fossasia/openevent/general/similarEvents/EventTopicsDao.kt

+/*
+    @Query("DELETE FROM EventTopic")
+    fun deleteAllEventTopics()
+*/


I thought I might probably require these methods in future, will probably remove them for now 👍

yes please remove this dont keep commented code in final version

simarsingh24 · 2018-06-25T05:10:59Z

app/src/main/java/org/fossasia/openevent/general/similarEvents/SimilarEventsFragment.kt

+        }
+
+        similarEventsRecyclerAdapter.setListener(recyclerViewClickListener)
+


extra line feed

simarsingh24 · 2018-06-25T05:11:22Z

app/src/main/java/org/fossasia/openevent/general/similarEvents/SimilarEventsViewModel.kt

+        super.onCleared()
+        compositeDisposable.clear()
+    }
+


simarsingh24 · 2018-06-25T05:12:07Z

good work btw 👍

nikit19 · 2018-06-25T07:18:23Z

app/src/main/java/org/fossasia/openevent/general/similarEvents/SimilarEventsViewModel.kt

+    val error = MutableLiveData<String>()
+
+    fun loadSimilarEvents(id: Long) {
+        if(id.toString() == "-1"){


why a conversion to string ? You can compare that directly

It's not working if I use id.equals(-1).

why dont you do id == -1

id is of type long actually, so an error is cropping up

Use id == -1

dreadpool2 · 2018-06-26T22:01:09Z

@iamareebjamal Could you please review this as I want to work on #207 after this

iamareebjamal · 2018-06-27T05:30:39Z

app/src/main/java/org/fossasia/openevent/general/event/Event.kt

+        val favorite: Boolean = false,
+        @ColumnInfo(index = true)
+        @Relationship("event-topic")
+        var eventTopic: EventTopicId? = null


Why not load the entire event topic in one go?

This way you can even show the event topic's name

Great! I will do that 👍

iamareebjamal · 2018-06-27T05:31:46Z

app/src/main/java/org/fossasia/openevent/general/event/EventDetailsFragment.kt

        rootView.endsOn.text = EventUtils.getFormattedDate(endsAt)

+        //Set event topic id
+        if(event.eventTopic != null){


Space around symbols

iamareebjamal · 2018-06-27T05:32:12Z

app/src/main/java/org/fossasia/openevent/general/event/EventDetailsFragment.kt


+        //Set event topic id
+        if(event.eventTopic != null){
+            eventTopicId = (event.eventTopic)?.id


No need for parenthesis

iamareebjamal · 2018-06-27T05:33:32Z

app/src/main/java/org/fossasia/openevent/general/event/EventDetailsFragment.kt

+        bundle.putLong(EVENT_ID, eventId)
+        eventTopicId?.let { bundle.putLong(EVENT_TOPIC_ID, it) }
+        similarEventsFragment.arguments = bundle
+        val transaction = childFragmentManager.beginTransaction()


No need of storing transaction in a variable

iamareebjamal · 2018-06-27T05:35:26Z

app/src/main/java/org/fossasia/openevent/general/similarEvents/EventTopicIdConverter.kt

+class EventTopicIdConverter {
+
+    @TypeConverter
+    fun fromEventTopicId(eventTopicId: EventTopicId?): Long?{


Space before {

iamareebjamal · 2018-06-27T05:35:42Z

app/src/main/java/org/fossasia/openevent/general/similarEvents/EventTopicService.kt

+import org.fossasia.openevent.general.event.Event
+import org.fossasia.openevent.general.event.EventDao
+
+class EventTopicService(private val eventApi: EventTopicApi, private val eventTopicsDao: EventTopicsDao, private val eventDao: EventDao) {


Property per line

iamareebjamal · 2018-06-27T05:36:30Z

app/src/main/java/org/fossasia/openevent/general/similarEvents/EventTopicService.kt

+                eventApi.getEventsUnderTopicId(id)
+                        .toFlowable()
+                        .map {
+                            eventTopicsDao.insertSimilarEvents(it)


How is this different from event service's method?

iamareebjamal · 2018-06-27T05:37:01Z

app/src/main/java/org/fossasia/openevent/general/similarEvents/EventTopicService.kt

+        }
+    }
+
+    fun setFavorite(eventId: Long, favourite: Boolean): Completable {


Why create unnecessary duplicate methods?

iamareebjamal · 2018-06-27T05:38:11Z

app/src/main/java/org/fossasia/openevent/general/similarEvents/EventTopicsDao.kt

+    fun deleteAllEvents()
+
+    @Query("SELECT * from Event WHERE eventTopic = :topicId")
+    fun getAllSimilarEvents(topicId: Long): Flowable<List<Event>>


Add this in EventDao, you are selecting from the same table, so should remain in the same DAO

By this logic, we should have FavoriteDao as well

dreadpool2 · 2018-06-27T08:33:56Z

PR Updated 👍

iamareebjamal · 2018-06-27T08:53:42Z

app/src/main/java/org/fossasia/openevent/general/similarEvents/EventTopicApi.kt

 interface EventTopicApi {

-    @GET("event-topics/{id}/events")
+    @GET("event-topics/{id}/events?include=event-topic&fields[event-topic]=id&page[size]=0")


No need to restrict the event topic field now

iamareebjamal · 2018-06-27T08:54:46Z

app/src/main/java/org/fossasia/openevent/general/similarEvents/EventTopicsDao.kt

+    @Query("DELETE FROM Event")
+    fun deleteAllEvents()
+
+}


No need of this file

Won't we require this in future to show users a list of event topics ?

iamareebjamal · 2018-06-27T08:55:04Z

app/src/main/java/org/fossasia/openevent/general/similarEvents/EventTopic.kt

+        @Id(LongIdHandler::class)
+        val id: Long? = null,
+        val name: String? = null,
+        val slug: String? = null


Add a foreign key to event

Actually EventTopic is not being inserted into the database, so I thought that foriegn key is not needed. Is it wrong ?

Okay! Got it wrong 👍

You have to insert it into database as well

Updated PR 👍

iamareebjamal · 2018-06-27T09:31:55Z

app/src/main/java/org/fossasia/openevent/general/similarEvents/EventTopic.kt

+        @PrimaryKey
+        val id: Long?,
+        val name: String?,
+        val slug: String?


There's no eventId to make foreign key of

dreadpool2 · 2018-06-27T09:54:02Z

@iamareebjamal Did all the changes requested 👍

iamareebjamal · 2018-06-27T10:03:43Z

app/src/main/java/org/fossasia/openevent/general/similarEvents/EventTopicsDao.kt

+    @Query("DELETE FROM Event")
+    fun deleteAllEvents()
+
+}


No need of this file

dreadpool2 · 2018-06-27T10:18:31Z

Removed EventTopicsDao for while 👍

iamareebjamal · 2018-06-27T10:21:40Z

app/src/main/java/org/fossasia/openevent/general/event/EventTopic.kt

-
-@Type("event-topic")
-@JsonNaming(PropertyNamingStrategy.KebabCaseStrategy::class)
-data class EventTopic(


Why did you remove this?

I had removed EventTopic as it was not needed that time. I added it again now 👍

iamareebjamal · 2018-06-27T10:22:12Z

app/src/main/java/org/fossasia/openevent/general/similarEvents/EventTopic.kt

@@ -0,0 +1,25 @@
+package org.fossasia.openevent.general.similarEvents


Package name doesn't have camel case

Changed it to similar_events

iamareebjamal · 2018-06-27T10:32:07Z

app/src/main/java/org/fossasia/openevent/general/event/EventUtils.kt

 object EventUtils {

+    val SIMILAR_EVENTS: String = "similarEvents"
+    val EVENTS: String = "events"


Where are these needed?

These are needed in EventsFragment and SimilarEventsFragment respectively. I have used the same adapter and ViewHolder for both the recycler views.

dreadpool2 · 2018-06-27T10:38:08Z

app/src/main/java/org/fossasia/openevent/general/event/EventsFragment.kt


+    override fun onCreate(savedInstanceState: Bundle?) {
+        super.onCreate(savedInstanceState)
+        eventsRecyclerAdapter.setEventLayout(EventUtils.EVENTS)


@iamareebjamal Here is one case

Use them locally then, no need for these in EventUtils

Did the changes 👍

dreadpool2 · 2018-06-27T10:39:11Z

app/src/main/java/org/fossasia/openevent/general/similar_events/SimilarEventsFragment.kt

+
+    override fun onCreate(savedInstanceState: Bundle?) {
+        super.onCreate(savedInstanceState)
+        similarEventsRecyclerAdapter.setEventLayout(EventUtils.SIMILAR_EVENTS)


Here is another case

Underscore is also not allowed, move the package in event package and rename to topic

Changed package name 👍

iamareebjamal · 2018-06-27T12:13:04Z

app/src/main/java/org/fossasia/openevent/general/event/EventDetailsFragment.kt


 class EventDetailsFragment : Fragment() {
    val EVENT_ID = "EVENT_ID"
+    val EVENT_TOPIC_ID = "EVENT_TOPIC_ID"


private const val and above as well

iamareebjamal · 2018-06-27T12:13:41Z

app/src/main/java/org/fossasia/openevent/general/event/EventsRecyclerAdapter.kt

    private var clickListener: RecyclerViewClickListener? = null
    private var favoriteFabListener: FavoriteFabListener? = null
+    private var eventLayout: String? = null
+    val SIMILAR_EVENTS: String = "similarEvents"


private const val

iamareebjamal · 2018-06-27T12:14:16Z

app/src/main/java/org/fossasia/openevent/general/event/topic/SimilarEventsFragment.kt

+    private val similarEventsViewModel by viewModel<SimilarEventsViewModel>()
+    private lateinit var rootView: View
+    private var EVENT_TOPIC_ID: String = "EVENT_TOPIC_ID"
+    val SIMILAR_EVENTS: String = "similarEvents"


iamareebjamal

Follow up PR to save the loaded event topic in DB

iamareebjamal · 2018-06-27T17:32:34Z

@dreadpool2 Squash your commits

dreadpool2 · 2018-06-27T17:32:55Z

@iamareebjamal Thanks! I will do that 👍

dreadpool2 · 2018-06-27T19:11:29Z

@iamareebjamal Rebased my commits 👍

iamareebjamal reviewed Jun 24, 2018

View reviewed changes

simarsingh24 reviewed Jun 25, 2018

View reviewed changes

nikit19 reviewed Jun 25, 2018

View reviewed changes

dreadpool2 mentioned this pull request Jun 26, 2018

Filter events based on location in similar events section #207

Closed

iamareebjamal reviewed Jun 27, 2018

View reviewed changes

iamareebjamal requested changes Jun 27, 2018

View reviewed changes

iamareebjamal reviewed Jun 27, 2018

View reviewed changes

dreadpool2 commented Jun 27, 2018

View reviewed changes

iamareebjamal reviewed Jun 27, 2018

View reviewed changes

iamareebjamal previously approved these changes Jun 27, 2018

View reviewed changes

fossasia deleted a comment Jun 27, 2018

dreadpool2 dismissed iamareebjamal’s stale review via cf05daa June 27, 2018 17:44

fossasia deleted a comment Jun 27, 2018

Sanyog Ghosh and others added 2 commits June 28, 2018 00:31

fix: Show similar events based on event topic

145a4cb

Merge branch 'development' into similar_events

f8d177c

iamareebjamal approved these changes Jun 27, 2018

View reviewed changes

iamareebjamal merged commit e3c0ad5 into fossasia:development Jun 27, 2018

		@@ -0,0 +1,11 @@
		package org.fossasia.openevent.general.similar

		}

		similarEventsRecyclerAdapter.setListener(recyclerViewClickListener)

		@@ -0,0 +1,25 @@
		package org.fossasia.openevent.general.similarEvents

feat: Show similar events based on event topic #198

feat: Show similar events based on event topic #198

Uh oh!

Conversation

dreadpool2 commented Jun 24, 2018

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

simarsingh24 Jun 25, 2018 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

simarsingh24 commented Jun 25, 2018

Uh oh!

Choose a reason for hiding this comment

Uh oh!

dreadpool2 Jun 25, 2018 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

dreadpool2 commented Jun 26, 2018

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

dreadpool2 commented Jun 27, 2018

Uh oh!

Choose a reason for hiding this comment

Uh oh!

simarsingh24 Jun 25, 2018 •

edited

Loading

dreadpool2 Jun 25, 2018 •

edited

Loading

dreadpool2 Jun 27, 2018 •

edited

Loading