Skip to content

Conversation

@aggarwalpulkit596
Copy link
Contributor

@aggarwalpulkit596 aggarwalpulkit596 commented Apr 14, 2019

Fixes #1604

Changes:Added Speakers section in Event Details
[WIP]Add a speaker page

Screenshots for the change:
screenshot-1555242469599

@aggarwalpulkit596 aggarwalpulkit596 force-pushed the speakers branch 2 times, most recently from 1fcfead to c076d7c Compare April 14, 2019 10:40
@aggarwalpulkit596 aggarwalpulkit596 changed the title [WIP]feat:add Speakers Section and Speakers Fragment feat:add Speakers Section and Speakers Fragment Apr 14, 2019
@auto-label auto-label bot added the feature label Apr 14, 2019
@fossasia fossasia deleted a comment Apr 14, 2019
@fossasia fossasia deleted a comment Apr 14, 2019
@aggarwalpulkit596
Copy link
Contributor Author

@iamareebjamal @liveHarshit please review

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.

IMO, no need to store in offline database

@iamareebjamal
Copy link
Member

This can be stored

@aggarwalpulkit596
Copy link
Contributor Author

We'll be a needing a speakerswithevent table

@aggarwalpulkit596
Copy link
Contributor Author

Because same speaker can be in two different events and it will make things complicated

@iamareebjamal
Copy link
Member

It won't

Copy link
Contributor Author

@aggarwalpulkit596 aggarwalpulkit596 left a comment

Choose a reason for hiding this comment

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

It won't

@iamareebjamal so should i use the table i created or make it work like event-topic

@aggarwalpulkit596
Copy link
Contributor Author

I tried to save information in a 3rd table and accordingly get back result as list of live data instead of flowable list i was able to save them but couldn't figure how to return that saved data

@iamareebjamal
Copy link
Member

Will take a look

@aggarwalpulkit596
Copy link
Contributor Author

@iamareebjamal any update ?

@aggarwalpulkit596
Copy link
Contributor Author

@liveHarshit please review now

@aggarwalpulkit596
Copy link
Contributor Author

@liveHarshit review this one as well

@liveHarshit
Copy link
Member

lgtm

@iamareebjamal iamareebjamal changed the title feat:add Speakers Section and Speakers Fragment feat: Add Speakers Section and Speakers Fragment Apr 16, 2019
@aggarwalpulkit596 aggarwalpulkit596 force-pushed the speakers branch 2 times, most recently from 9aeb6ce to aa0735b Compare April 17, 2019 08:07
@aggarwalpulkit596
Copy link
Contributor Author

@iamareebjamal updated

@aggarwalpulkit596 aggarwalpulkit596 changed the title feat: Add Speakers Section and Speakers Fragment feat: Add Speakers Section Apr 17, 2019
@iamareebjamal
Copy link
Member

Everything working as expected?

@aggarwalpulkit596
Copy link
Contributor Author

@iamareebjamal yup

iamareebjamal
iamareebjamal previously approved these changes Apr 17, 2019
}

fun loadEventSpeakers(id: Long): LiveData<List<Speaker>> {
val speakerList = speakerService.fetchSpeakersFromDb(id)
Copy link
Member

Choose a reason for hiding this comment

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

Store this in the ViewModel. What use is this variable, you're just returning it

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.

Rest looks good

@aggarwalpulkit596
Copy link
Contributor Author

@iamareebjamal updated

@aggarwalpulkit596
Copy link
Contributor Author

@nikit19 please review now

@iamareebjamal
Copy link
Member

schema JSON is not updated

@aggarwalpulkit596
Copy link
Contributor Author

@iamareebjamal updated JSON Schema and resolved conflicts

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

Labels

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Add Speaker Section

4 participants