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.

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: Add List operation implementation for Sponsors Endpoint. #900 #901

Merged
merged 4 commits into from May 11, 2018

Conversation

5 participants
@mishuvs
Member

mishuvs commented Apr 15, 2018

Fixes #900

Changes: [Add here what changes were made in this issue and if possible provide links.]

  1. Implemented Sponsors module
  2. Now, fetches Sponsors for an Event
  3. Saves it in the database for offline use
  4. Shows it in a RecyclerView

Gif for the change:
videotogif_2018 05 11_07 14 07

@iamareebjamal

This comment has been minimized.

Show comment
Hide comment
@iamareebjamal

iamareebjamal Apr 15, 2018

Member

Also show the image of the sponsor

Member

iamareebjamal commented Apr 15, 2018

Also show the image of the sponsor

@codecov

This comment has been minimized.

Show comment
Hide comment
@codecov

codecov bot Apr 15, 2018

Codecov Report

Merging #901 into development will decrease coverage by 0.84%.
The diff coverage is 0%.

Impacted file tree graph

@@                Coverage Diff                @@
##             development     #901      +/-   ##
=================================================
- Coverage          33.02%   32.17%   -0.85%     
  Complexity           540      540              
=================================================
  Files                145      151       +6     
  Lines               4436     4553     +117     
  Branches             151      152       +1     
=================================================
  Hits                1465     1465              
- Misses              2908     3025     +117     
  Partials              63       63
Impacted Files Coverage Δ Complexity Δ
...nevent/app/core/sponsor/list/SponsorsFragment.java 0% <0%> (ø) 0 <0> (?)
...re/sponsor/list/viewholder/SponsorsViewHolder.java 0% <0%> (ø) 0 <0> (?)
...ent/app/core/sponsor/list/SponsorsListAdapter.java 0% <0%> (ø) 0 <0> (?)
...nevent/app/data/sponsor/SponsorRepositoryImpl.java 0% <0%> (ø) 0 <0> (?)
...event/app/core/sponsor/list/SponsorsPresenter.java 0% <0%> (ø) 0 <0> (?)
...g/fossasia/openevent/app/data/sponsor/Sponsor.java 0% <0%> (ø) 0 <0> (?)
...sia/openevent/app/core/main/FragmentNavigator.java 50% <0%> (-2.39%) 6 <0> (ø)
... and 3 more

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update 1f7a303...73b0c01. Read the comment docs.

codecov bot commented Apr 15, 2018

Codecov Report

Merging #901 into development will decrease coverage by 0.84%.
The diff coverage is 0%.

Impacted file tree graph

@@                Coverage Diff                @@
##             development     #901      +/-   ##
=================================================
- Coverage          33.02%   32.17%   -0.85%     
  Complexity           540      540              
=================================================
  Files                145      151       +6     
  Lines               4436     4553     +117     
  Branches             151      152       +1     
=================================================
  Hits                1465     1465              
- Misses              2908     3025     +117     
  Partials              63       63
Impacted Files Coverage Δ Complexity Δ
...nevent/app/core/sponsor/list/SponsorsFragment.java 0% <0%> (ø) 0 <0> (?)
...re/sponsor/list/viewholder/SponsorsViewHolder.java 0% <0%> (ø) 0 <0> (?)
...ent/app/core/sponsor/list/SponsorsListAdapter.java 0% <0%> (ø) 0 <0> (?)
...nevent/app/data/sponsor/SponsorRepositoryImpl.java 0% <0%> (ø) 0 <0> (?)
...event/app/core/sponsor/list/SponsorsPresenter.java 0% <0%> (ø) 0 <0> (?)
...g/fossasia/openevent/app/data/sponsor/Sponsor.java 0% <0%> (ø) 0 <0> (?)
...sia/openevent/app/core/main/FragmentNavigator.java 50% <0%> (-2.39%) 6 <0> (ø)
... and 3 more

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update 1f7a303...73b0c01. Read the comment docs.

@mishuvs

This comment has been minimized.

Show comment
Hide comment
@mishuvs

mishuvs Apr 15, 2018

Member

There's a field url, which probably, is the URL of sponsor's website. I am not sure if it's image URL field. Should I use this for image's URL?

Member

mishuvs commented Apr 15, 2018

There's a field url, which probably, is the URL of sponsor's website. I am not sure if it's image URL field. Should I use this for image's URL?

@iamareebjamal

This comment has been minimized.

Show comment
Hide comment
@iamareebjamal

iamareebjamal Apr 15, 2018

Member

There's logo-url as well

Member

iamareebjamal commented Apr 15, 2018

There's logo-url as well

@mishuvs

This comment has been minimized.

Show comment
Hide comment
@mishuvs
Member

mishuvs commented Apr 15, 2018

@iamareebjamal

This comment has been minimized.

Show comment
Hide comment
@iamareebjamal

iamareebjamal Apr 15, 2018

Member

It is there, look in sponsor details

Member

iamareebjamal commented Apr 15, 2018

It is there, look in sponsor details

@srv-twry

This comment has been minimized.

Show comment
Hide comment
@srv-twry

srv-twry May 2, 2018

Member

@mishuvs Please update the PR 😄
The more time you take, higher will be the number of conflicts.

Member

srv-twry commented May 2, 2018

@mishuvs Please update the PR 😄
The more time you take, higher will be the number of conflicts.

@mishuvs mishuvs changed the title from feat: Add List operation implementation for Sponsors Endpoint. #900 to [WIP] feat: Add List operation implementation for Sponsors Endpoint. #900 May 6, 2018

@mishuvs

This comment has been minimized.

Show comment
Hide comment
@mishuvs

mishuvs May 6, 2018

Member

I apologise for causing the delay. I got caught up in my end-sem exams. I will continue work on this after my exam tomorrow.
Thanks for the suggestions!

Member

mishuvs commented May 6, 2018

I apologise for causing the delay. I got caught up in my end-sem exams. I will continue work on this after my exam tomorrow.
Thanks for the suggestions!

@fossasia fossasia deleted a comment from codacy-bot May 8, 2018

@fossasia fossasia deleted a comment from codacy-bot May 8, 2018

@mishuvs mishuvs changed the title from [WIP] feat: Add List operation implementation for Sponsors Endpoint. #900 to feat: Add List operation implementation for Sponsors Endpoint. #900 May 8, 2018

@mishuvs

This comment has been minimized.

Show comment
Hide comment
@mishuvs

mishuvs May 9, 2018

Member

The list doesn't load automatically and ends up empty unless I put forceReload to true or refresh manually. And the headers sometimes show up and sometimes don't, I am trying to figure out why.
videotogif_2018 05 09_07 22 44

Member

mishuvs commented May 9, 2018

The list doesn't load automatically and ends up empty unless I put forceReload to true or refresh manually. And the headers sometimes show up and sometimes don't, I am trying to figure out why.
videotogif_2018 05 09_07 22 44

@iamareebjamal

Great job

Show outdated Hide outdated app/src/main/java/org/fossasia/openevent/app/data/sponsor/Sponsor.java
Show outdated Hide outdated app/src/main/java/org/fossasia/openevent/app/data/sponsor/Sponsor.java
Show outdated Hide outdated app/src/main/java/org/fossasia/openevent/app/data/sponsor/Sponsor.java
Show outdated Hide outdated app/src/main/java/org/fossasia/openevent/app/data/sponsor/Sponsor.java
.reload(reload)
.withDiskObservable(diskObservable)
.withNetworkObservable(networkObservable)
.build();

This comment has been minimized.

@iamareebjamal

iamareebjamal May 9, 2018

Member

I suspect the problem is here. Can you check the logs

@iamareebjamal

iamareebjamal May 9, 2018

Member

I suspect the problem is here. Can you check the logs

This comment has been minimized.

@mishuvs

mishuvs May 9, 2018

Member

It works when toSortedList() is changed to toList() in the presenter's loadSponsor().
The list is loading but data is not getting bind when this is done. I had also tried this before removing the Delegate and it didn't work even then.
So currently on the first click, there are empty items, and data is bound only on refresh.
I am not able to find anything about this in the logs.
device-2018-05-09-211021

@iamareebjamal @srv-twry @Masquerade0097 could you guys please guide me on this one?

@mishuvs

mishuvs May 9, 2018

Member

It works when toSortedList() is changed to toList() in the presenter's loadSponsor().
The list is loading but data is not getting bind when this is done. I had also tried this before removing the Delegate and it didn't work even then.
So currently on the first click, there are empty items, and data is bound only on refresh.
I am not able to find anything about this in the logs.
device-2018-05-09-211021

@iamareebjamal @srv-twry @Masquerade0097 could you guys please guide me on this one?

This comment has been minimized.

@iamareebjamal

iamareebjamal May 10, 2018

Member

OK, first resolve the conflicts so we can locally pull the PR

@iamareebjamal

iamareebjamal May 10, 2018

Member

OK, first resolve the conflicts so we can locally pull the PR

@fossasia fossasia deleted a comment from codacy-bot May 9, 2018

@fossasia fossasia deleted a comment from codacy-bot May 9, 2018

@fossasia fossasia deleted a comment from codacy-bot May 9, 2018

@fossasia fossasia deleted a comment from codacy-bot May 9, 2018

robuxinc added some commits May 10, 2018

Merge branch 'development' of https://github.com/fossasia/open-event-…
…orga-app into list-sponsors#900

# Conflicts:
#	app/src/main/java/org/fossasia/openevent/app/common/di/module/android/MainFragmentBuildersModule.java
@mishuvs

This comment has been minimized.

Show comment
Hide comment
@mishuvs

mishuvs May 11, 2018

Member

Sorry for the trouble guys. I think the fields weren't getting stored in the database. 😅 But if that's the case I still don't get why then all fields showed up at a refresh. 😕
@iamareebjamal Please review.

Member

mishuvs commented May 11, 2018

Sorry for the trouble guys. I think the fields weren't getting stored in the database. 😅 But if that's the case I still don't get why then all fields showed up at a refresh. 😕
@iamareebjamal Please review.

@iamareebjamal

This comment has been minimized.

Show comment
Hide comment
@iamareebjamal

iamareebjamal May 11, 2018

Member

Because that was being loaded from network directly I guess

Member

iamareebjamal commented May 11, 2018

Because that was being loaded from network directly I guess

@iamareebjamal

This comment has been minimized.

Show comment
Hide comment
@iamareebjamal

iamareebjamal May 11, 2018

Member

Is the UI same as reported previously?

Member

iamareebjamal commented May 11, 2018

Is the UI same as reported previously?

@mishuvs

This comment has been minimized.

Show comment
Hide comment
@mishuvs

mishuvs May 11, 2018

Member

Previously? Headers were removed as you had asked for.
I have updated the gif in the PR description.

Member

mishuvs commented May 11, 2018

Previously? Headers were removed as you had asked for.
I have updated the gif in the PR description.

@iamareebjamal

Nice UI. Good job

@open-event-bot open-event-bot bot removed the needs-review label May 11, 2018

@iamareebjamal iamareebjamal merged commit f2276d0 into fossasia:development May 11, 2018

2 of 5 checks passed

Codacy/PR Quality Review Codacy was unable to analyse your pull request.
Details
codecov/patch 0% of diff hit (target 33.02%)
Details
codecov/project 32.17% (-0.85%) compared to 1f7a303
Details
ci/circleci Your tests passed on CircleCI!
Details
continuous-integration/travis-ci/pr The Travis CI build passed
Details

@mishuvs mishuvs deleted the mishuvs:list-sponsors#900 branch May 11, 2018

iamareebjamal added a commit that referenced this pull request Jul 16, 2018

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment