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.

Sign up for GitHub

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

Jump to bottom

feat: Fetch settings and divide events view model #2104

Merged
merged 1 commit into from
Jul 15, 2019
Merged

feat: Fetch settings and divide events view model #2104

merged 1 commit into from
Jul 15, 2019

Conversation

liveHarshit
Copy link
Member

@liveHarshit liveHarshit commented Jul 11, 2019 •
edited
Loading

Fixes #2101
Fixes #2083

@auto-label auto-label bot added the fix label Jul 11, 2019
@liveHarshit liveHarshit mentioned this pull request Jul 12, 2019
Closed
@liveHarshit liveHarshit added the on hold: server side Needs to solve on the server first label Jul 12, 2019
@liveHarshit liveHarshit changed the title fix: Order expiry time fix: Fetch order expiry time from settings Jul 12, 2019
@liveHarshit liveHarshit removed the on hold: server side Needs to solve on the server first label Jul 12, 2019
@liveHarshit liveHarshit changed the title fix: Fetch order expiry time from settings feat: Fetch settings and divide events view model Jul 12, 2019
@auto-label auto-label bot added feature and removed fix labels Jul 12, 2019
@liveHarshit
Copy link
Member Author

Updated

iamareebjamal
iamareebjamal reviewed Jul 12, 2019
View reviewed changes
app/src/main/java/org/fossasia/openevent/general/attendees/AttendeeFragment.kt
}
})

attendeeViewModel.getSettings()
Copy link
Member

Choose a reason for hiding this comment

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

There is no guarantee that settings have been fetched till the next statement is executed

Copy link
Member Author

Choose a reason for hiding this comment

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

Then onErrorResumeNext will execute and settings will fetch from api.

Copy link
Member

@iamareebjamal iamareebjamal Jul 12, 2019 •
edited
Loading

Choose a reason for hiding this comment

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

Still there is no guarantee that settings have been fetched till the next statement is executed, as getSettings is asynchronous

Copy link
Member Author

Choose a reason for hiding this comment

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

Sorry, I don't understand which next statement. During fetching/getting settings progress dialog is displaying and if settings fetch successfully or there is an error(=15), value for mutableOrderExpiryTime is set and only then countdown timer will start.

Copy link
Member

Choose a reason for hiding this comment

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

The statement which is immediately proceeding getSettings

@liveHarshit liveHarshit mentioned this pull request Jul 13, 2019
Merged
@liveHarshit
Copy link
Member Author

Updated

@iamareebjamal
Copy link
Member

Approving but there is a need of polishing

@iamareebjamal iamareebjamal merged commit 1e5072d into fossasia:development Jul 15, 2019
@liveHarshit liveHarshit deleted the 2101 branch July 15, 2019 12:33
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@anhanh11001 anhanh11001 anhanh11001 left review comments

@iamareebjamal iamareebjamal iamareebjamal approved these changes

Assignees
No one assigned
Labels
feature
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.

Fetch timer from settings Break events view model into smaller ones
3 participants
@liveHarshit @iamareebjamal @anhanh11001