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.

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

fix: Update check-in system to work properly #1673

Merged
merged 4 commits into from
May 28, 2019
Merged

fix: Update check-in system to work properly #1673

merged 4 commits into from
May 28, 2019

Conversation

ShridharGoel
Copy link
Member

@ShridharGoel ShridharGoel commented May 27, 2019

Fixes #1672

Checklist:

  • I have checked for PMD and check-style issues
  • I have read the Contribution & Best practices Guide and my PR follows them.
  • My branch is up-to-date with the Upstream development branch.

Changes:

  • Proper attendee IDs are being set.
  • Attendee check-in status gets updated after scanning.

@auto-label auto-label bot added the fix label May 27, 2019
@iamareebjamal
Copy link
Member

Please summarize the changes and explain why it wasn't working and how you fixed it

@ShridharGoel
Copy link
Member Author

ShridharGoel commented May 27, 2019

The getAttendeesPagewise() method fetches the list at a particular page and appends it to the earlier list which was fetched till then. So, the updateLocal() method wasn't taking the correct list, it was using attendeesList in the presenter which had the list of attendees of just one page and not the complete list. So, I have created getAttendeeList() in AttendeesFragment and replaced attendeeList with getView().getAttendeeList() in AttendeesPresenter.

Apart from this, after scanning, the list was not updating until refreshed. Therefore, I've added this in the ScanQRPresenter:

attendee.isCheckedIn = !attendee.isCheckedIn;
attendeeRepository.scheduleToggle(attendee)
    .subscribe(() -> {
        // Nothing to do
    }, Logger::logError);

@ShridharGoel
Copy link
Member Author

One issue that still exists: The attendee IDs do not update when the fragment is opened for the first time. After refreshing once, the correct values show. This is because the attendees are not getting stored in the local database until refreshed once. Although we are shifting to Room and that would probably fix this, but I think this needs to be fixed right now. So, I'm working on it.
@iamareebjamal Please tell if you have any suggestions on this.

@iamareebjamal
Copy link
Member

I'm not able to understand why this part was needed

attendeeRepository.scheduleToggle(attendee)
    .subscribe(() -> {
        // Nothing to do
    }, Logger::logError);

@ShridharGoel
Copy link
Member Author

I'm not able to understand why this part was needed

attendeeRepository.scheduleToggle(attendee)
    .subscribe(() -> {
        // Nothing to do
    }, Logger::logError);

To update the attendee in the local DB?

@NonNull
    public Completable scheduleToggle(Attendee attendee) {
        return repository
            .update(Attendee.class, attendee)
            .concatWith(completableObserver -> {
                AttendeeCheckInWork.scheduleWork();
                if (!repository.isConnected())
                    completableObserver.onError(new Exception("No network present. Added to job queue"));
            })
            .subscribeOn(Schedulers.io());
    }

@ShridharGoel
Copy link
Member Author

attendee.isCheckedIn = !attendee.isCheckedIn;

This is not needed since it is being handled here:

if (toCheckIn) {
            getView().showMessage(
                attendee.isCheckedIn ? R.string.already_checked_in : R.string.now_checked_in,
                true
            );
            attendee.isCheckedIn = true;
        } else if (toCheckOut) {
            getView().showMessage(
                attendee.isCheckedIn ? R.string.now_checked_out : R.string.already_checked_out,
                true
            );
            attendee.isCheckedIn = false;
        }

@iamareebjamal
Copy link
Member

iamareebjamal commented May 28, 2019

But it was working before without it. Why is it needed here, now?

@ShridharGoel
Copy link
Member Author

ShridharGoel commented May 28, 2019

But it was working before without it. Why is it needed now?

Wasn't working, the attendee status did not change until the list was refreshed.

@iamareebjamal
Copy link
Member

Again, before the pagination changes, it was working. You can check in the old APK. It means this was triggered somewhere else as well. Please check the reason why it stopped working

@ShridharGoel
Copy link
Member Author

Okay, checking it.

@ShridharGoel
Copy link
Member Author

Scanning is working as expected. Earlier, I think I was pressing the back button quickly after scanning to go back to the list so maybe that was the reason of the list not showing updated changes or maybe it could have been because of internet connectivity issue. Reverted the changes in ScanQRPresenter.

There was an issue because of which on loading second and further pages, the data of the earlier page was getting deleted from the local DB, due to which attendee IDs started showing as 0 for the earlier pages. Updated the AttendeeRepositoryImpl to fix this.

@ShridharGoel
Copy link
Member Author

One issue that still exists: The attendee IDs do not update when the fragment is opened for the first time. After refreshing once, the correct values show. This is because the attendees are not getting stored in the local database until refreshed once. Although we are shifting to Room and that would probably fix this, but I think this needs to be fixed right now. So, I'm working on it.
@iamareebjamal Please tell if you have any suggestions on this.

This is also fixed.

@iamareebjamal iamareebjamal merged commit e0d2a92 into fossasia:development May 28, 2019
mariobehling pushed a commit that referenced this pull request Jun 11, 2019
* build(deps): bump appcompat from 1.1.0-alpha04 to 1.1.0-alpha05 (#1634)

Bumps appcompat from 1.1.0-alpha04 to 1.1.0-alpha05.

Signed-off-by: dependabot[bot] <support@dependabot.com>

* build(deps): bump lifecycle-compiler from 2.1.0-alpha04 to 2.2.0-alpha01 (#1633)

Bumps lifecycle-compiler from 2.1.0-alpha04 to 2.2.0-alpha01.

Signed-off-by: dependabot[bot] <support@dependabot.com>

* build(deps): bump recyclerview from 1.1.0-alpha04 to 1.1.0-alpha05 (#1630)

Bumps recyclerview from 1.1.0-alpha04 to 1.1.0-alpha05.

Signed-off-by: dependabot[bot] <support@dependabot.com>

* build(deps): bump annotation from 1.1.0-beta01 to 1.1.0-rc01 (#1628)

Bumps annotation from 1.1.0-beta01 to 1.1.0-rc01.

Signed-off-by: dependabot[bot] <support@dependabot.com>

* build(deps): bump lifecycle-common-java8 from 2.1.0-alpha04 to 2.2.0-alpha01 (#1631)

Bumps lifecycle-common-java8 from 2.1.0-alpha04 to 2.2.0-alpha01.

Signed-off-by: dependabot[bot] <support@dependabot.com>

* fix: Remove sticky headers from Attendees fragment (#1636)

* build(deps): bump lifecycle-extensions from 2.1.0-alpha04 to 2.2.0-alpha01 (#1629)

Bumps lifecycle-extensions from 2.1.0-alpha04 to 2.2.0-alpha01.

Signed-off-by: dependabot[bot] <support@dependabot.com>

* build(deps): bump core-testing from 2.1.0-alpha02 to 2.1.0-beta01 (#1635)

Bumps core-testing from 2.1.0-alpha02 to 2.1.0-beta01.

Signed-off-by: dependabot[bot] <support@dependabot.com>

* build(deps): bump lifecycle-runtime from 2.1.0-alpha04 to 2.2.0-alpha01 (#1632)

Bumps lifecycle-runtime from 2.1.0-alpha04 to 2.2.0-alpha01.

Signed-off-by: dependabot[bot] <support@dependabot.com>

* fix: Prevent closing of Edit Event without confirmation dialog (#1639)

* chore: Convert SalesSummaryPresenter to ViewModel (#1642)

* feat: Pagination in Attendees list (#1637)

* fix: Prevent crash at check-in on second page onwards (#1647)

* chore: Add unit tests for SalesSummaryViewModel (#1644)

* chore: Add unit tests for SalesSummaryViewModel

* chore: Add unit tests for SalesSummaryViewModel

* feat: Integrate Error Prone (#1582)

* feat: Integrate error prone

* Update Error Prone version

* build(deps): bump threetenbp from 1.3.8 to 1.4.0 (#1640)

Bumps [threetenbp](https://github.com/ThreeTen/threetenbp) from 1.3.8 to 1.4.0.
- [Release notes](https://github.com/ThreeTen/threetenbp/releases)
- [Changelog](https://github.com/ThreeTen/threetenbp/blob/master/RELEASE-NOTES.md)
- [Commits](ThreeTen/threetenbp@v1.3.8...v1.4.0)

Signed-off-by: dependabot[bot] <support@dependabot.com>

* build(deps): bump gradle from 3.4.0 to 3.4.1 (#1645)

Bumps gradle from 3.4.0 to 3.4.1.

Signed-off-by: dependabot[bot] <support@dependabot.com>

* chore: Update LeakCanary (#1649)

* fix: Refresh Sponsors list automatically on deletion of items (#1651)

* fix: Update Sponsors list automatically on creation of new item (#1652)

* build(deps): bump logging-interceptor from 3.14.1 to 3.14.2 (#1655)

Bumps logging-interceptor from 3.14.1 to 3.14.2.

Signed-off-by: dependabot[bot] <support@dependabot.com>

* chore: Migrate FeedbackListPresenter to ViewModel (#1657)

* chore: Migrate FeedbackListPresenter to ViewModel

* chore: Migrate FeedbackListPresenter to ViewModel

* chore: Migrate FaqListPresenter to ViewModel (#1659)

*  fix: Dismiss clear button when amount becomes zero (#1421)

* fix: Dismiss clear button when amount becomes zero

* fix: Dismiss clear button when amount becomes zero

* Fix style problem

* Add space after if

* Update README.md

* Delete .gitkeep

* fix: DatePicker and TimePicker doesn't display time chosen before when clicked (#1551)

Changes:
- Retain value of current time so in the AbstractDateTimePicker class

Fixes #1548

* fix: Prevent crash on refreshing Event Dashboard (#1675)

* fix: Update check-in system to work properly (#1673)

* fix: Update check-in system to work properly

* fix: Update check-in system to work properly

* fix: Update check-in system to work properly

* build(deps): bump mockito-inline from 2.27.0 to 2.28.1 (#1677)

Bumps [mockito-inline](https://github.com/mockito/mockito) from 2.27.0 to 2.28.1.
- [Release notes](https://github.com/mockito/mockito/releases)
- [Commits](mockito/mockito@v2.27.0...v2.28.1)

* build(deps): bump mockito-inline from 2.28.1 to 2.28.2 (#1679)

Bumps [mockito-inline](https://github.com/mockito/mockito) from 2.28.1 to 2.28.2.
- [Release notes](https://github.com/mockito/mockito/releases)
- [Commits](mockito/mockito@v2.28.1...v2.28.2)

Co-authored-by: null <dependabot-preview[bot]@users.noreply.github.com>

* build(deps): bump espresso-core from 3.2.0-beta01 to 3.2.0 (#1682)

Bumps espresso-core from 3.2.0-beta01 to 3.2.0.

Co-authored-by: null <dependabot-preview[bot]@users.noreply.github.com>

* build(deps): bump material from 1.1.0-alpha06 to 1.1.0-alpha07 (#1681)

Bumps [material](https://github.com/material-components/material-components-android) from 1.1.0-alpha06 to 1.1.0-alpha07.
- [Release notes](https://github.com/material-components/material-components-android/releases)
- [Commits](https://github.com/material-components/material-components-android/commits)

Co-authored-by: null <dependabot-preview[bot]@users.noreply.github.com>

* feat: Implement system of role invites (#1680)

* feat: Implement system of role invites

* feat: Implement system of role invites

* Update unit tests

* feat: Implement system of role invites

* feat: Implement system of role invites

* fix: Prevent unresponsive FAQ fragment issue (#1687)

* build(deps): bump gradle-errorprone-plugin from 0.8 to 0.8.1 (#1688)

Bumps gradle-errorprone-plugin from 0.8 to 0.8.1.

Co-authored-by: null <dependabot-preview[bot]@users.noreply.github.com>

* fix: Show correct state of map display checkbox (#1692)

* build(deps): bump recyclerview from 1.1.0-alpha05 to 1.1.0-alpha06 (#1697)

Bumps recyclerview from 1.1.0-alpha05 to 1.1.0-alpha06.

Signed-off-by: dependabot-preview[bot] <support@dependabot.com>

Co-authored-by: null <dependabot-preview[bot]@users.noreply.github.com>

* build(deps): bump annotation from 1.1.0-rc01 to 1.1.0 (#1695)

Bumps annotation from 1.1.0-rc01 to 1.1.0.

Signed-off-by: dependabot-preview[bot] <support@dependabot.com>

Co-authored-by: null <dependabot-preview[bot]@users.noreply.github.com>

* build(deps): bump appcompat from 1.1.0-alpha05 to 1.1.0-beta01 (#1696)

Bumps appcompat from 1.1.0-alpha05 to 1.1.0-beta01.

Signed-off-by: dependabot-preview[bot] <support@dependabot.com>

Co-authored-by: null <dependabot-preview[bot]@users.noreply.github.com>

* feat: Implement notifications section (#1694)

* feat: Implement notifications section

* Remove item divider

* Remove unused imports

* chore: Bump version code and name for release of v1.2.0 (#1703)
@ShridharGoel ShridharGoel deleted the fix-check-in branch June 19, 2019 06:41
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.

Check-in mechanism not working properly
2 participants