Skip to content
This repository has been archived by the owner on Apr 17, 2023. It is now read-only.

DataManger extensions #104

Merged
merged 2 commits into from
Aug 31, 2017
Merged

Conversation

wtrocki
Copy link
Member

@wtrocki wtrocki commented Aug 30, 2017

Motivation

Implement forceSync in DataManagers for sync

See: feedhenry-raincatcher/raincatcher-angularjs#64 for mobile changes.

@coveralls
Copy link

Coverage Status

Changes Unknown when pulling b569e5b on wtrocki:RAINCATCH-1126 into ** on feedhenry-raincatcher:master**.

public stop(): Bluebird<any> {
const self = this;
return new Bluebird(function(resolve, reject) {
syncApi.startSync(self.datasetId, function() {
Copy link
Contributor

Choose a reason for hiding this comment

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

this should be stopSync?

private getQueueSize = function() {
const self = this;
return new Bluebird(function(resolve) {
syncApi.getPending(self.datasetId, function(pending?) {
Copy link
Contributor

Choose a reason for hiding this comment

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

Whats the ?

Copy link
Member Author

Choose a reason for hiding this comment

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

short form of pending?:any

Copy link
Contributor

Choose a reason for hiding this comment

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

pending: any | undefined

Copy link
Contributor

Choose a reason for hiding this comment

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

Is there a way to handle errors for this?

Copy link
Member Author

Choose a reason for hiding this comment

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

Actually I was thinking about this as well and aparently this code is just fetching local data. Errors will just return empty object.

Copy link
Contributor

Choose a reason for hiding this comment

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

👍 I thought that might be the case, maybe of an error value like -1 or so.

@coveralls
Copy link

coveralls commented Aug 31, 2017

Coverage Status

Changes Unknown when pulling d5a53cf on wtrocki:RAINCATCH-1126 into ** on feedhenry-raincatcher:master**.

*/
public list(callback: (err?: Error, results?: any) => void) {
public list(): Bluebird<any> {
Copy link
Contributor

Choose a reason for hiding this comment

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

I think this could at least be an array type instead of bare any, helps client code a little.

Copy link
Member Author

Choose a reason for hiding this comment

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

To be clear - you mean any[] ?

Copy link
Contributor

Choose a reason for hiding this comment

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

Yes I think that works

resolve(dataSetData);
}, function handleSyncListError(syncErrorCode: string, syncErrorMessage: string) {
reject(new Error(self.formatSyncErrorMessage(syncErrorCode, syncErrorMessage)));
});
Copy link
Contributor

Choose a reason for hiding this comment

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

💯

Bluebird.resolve();
}, function() {
Bluebird.reject(new Error('forceSync error'));
});
Copy link
Contributor

Choose a reason for hiding this comment

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

I think this last then is redundant, and wrong since it's not returning anything.

If you're just looking to mask the return value and error value, .then(() => undefined).catch(() => Bluebird.reject(new Error('forceSync error'))) should do the trick.

paolobueno
paolobueno previously approved these changes Aug 31, 2017
@coveralls
Copy link

coveralls commented Aug 31, 2017

Coverage Status

Changes Unknown when pulling 21d84bc on wtrocki:RAINCATCH-1126 into ** on feedhenry-raincatcher:master**.

@wtrocki wtrocki merged commit 1a47c39 into feedhenry-raincatcher:master Aug 31, 2017
@wtrocki wtrocki deleted the RAINCATCH-1126 branch August 31, 2017 18:08
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Development

Successfully merging this pull request may close these issues.

None yet

4 participants