From a0c57f46c033aaf7949ac6bca5924bbfae054437 Mon Sep 17 00:00:00 2001 From: Albert Gasset Date: Wed, 21 Mar 2018 15:50:05 +0100 Subject: [PATCH] MOBILE-2319 notes: PR fixes --- src/addon/notes/notes.module.ts | 20 +--- src/addon/notes/pages/add/add.html | 2 +- src/addon/notes/pages/list/list.html | 7 +- src/addon/notes/pages/list/list.ts | 11 ++- .../notes/providers/course-option-handler.ts | 20 +--- src/addon/notes/providers/notes.ts | 98 +++---------------- src/addon/notes/providers/user-handler.ts | 12 ++- 7 files changed, 40 insertions(+), 130 deletions(-) diff --git a/src/addon/notes/notes.module.ts b/src/addon/notes/notes.module.ts index 4ea5dd0a8..7dd83dc35 100644 --- a/src/addon/notes/notes.module.ts +++ b/src/addon/notes/notes.module.ts @@ -22,11 +22,7 @@ import { AddonNotesUserHandler } from './providers/user-handler'; import { AddonNotesComponentsModule } from './components/components.module'; import { CoreCourseOptionsDelegate } from '@core/course/providers/options-delegate'; import { CoreCronDelegate } from '@providers/cron'; -import { CoreCoursesProvider } from '@core/courses/providers/courses'; -import { CoreEventsProvider } from '@providers/events'; -import { CoreSitesProvider } from '@providers/sites'; import { CoreUserDelegate } from '@core/user/providers/user-delegate'; -import { CoreUserProvider } from '@core/user/providers/user'; @NgModule({ declarations: [ @@ -45,23 +41,11 @@ import { CoreUserProvider } from '@core/user/providers/user'; }) export class AddonNotesModule { constructor(courseOptionsDelegate: CoreCourseOptionsDelegate, courseOptionHandler: AddonNotesCourseOptionHandler, - userDelegate: CoreUserDelegate, userHandler: AddonNotesUserHandler, cronDelegate: CoreCronDelegate, - syncHandler: AddonNotesSyncCronHandler, eventsProvider: CoreEventsProvider, sitesProvider: CoreSitesProvider) { + userDelegate: CoreUserDelegate, userHandler: AddonNotesUserHandler, + cronDelegate: CoreCronDelegate, syncHandler: AddonNotesSyncCronHandler) { // Register handlers. courseOptionsDelegate.registerHandler(courseOptionHandler); userDelegate.registerHandler(userHandler); cronDelegate.register(syncHandler); - - eventsProvider.on(CoreEventsProvider.LOGOUT, () => { - courseOptionHandler.clearCoursesNavCache(); - }, sitesProvider.getCurrentSiteId()); - - eventsProvider.on(CoreCoursesProvider.EVENT_MY_COURSES_REFRESHED, () => { - courseOptionHandler.clearCoursesNavCache(); - }, sitesProvider.getCurrentSiteId()); - - eventsProvider.on(CoreUserProvider.PROFILE_REFRESHED, () => { - userHandler.clearAddNoteCache(); - }, sitesProvider.getCurrentSiteId()); } } diff --git a/src/addon/notes/pages/add/add.html b/src/addon/notes/pages/add/add.html index 59b0bb32d..e0a41c7b1 100644 --- a/src/addon/notes/pages/add/add.html +++ b/src/addon/notes/pages/add/add.html @@ -21,7 +21,7 @@ - diff --git a/src/addon/notes/pages/list/list.html b/src/addon/notes/pages/list/list.html index 02d5f6084..1431ca259 100644 --- a/src/addon/notes/pages/list/list.html +++ b/src/addon/notes/pages/list/list.html @@ -17,9 +17,10 @@ -

- {{ 'core.thereisdatatosync' | translate:{$a: 'addon.notes.notes' | translate | lowercase } }} -

+
+ + {{ 'core.thereisdatatosync' | translate:{$a: 'addon.notes.notes' | translate | lowercase } }} +
diff --git a/src/addon/notes/pages/list/list.ts b/src/addon/notes/pages/list/list.ts index fcebe9a34..d64b578b9 100644 --- a/src/addon/notes/pages/list/list.ts +++ b/src/addon/notes/pages/list/list.ts @@ -76,10 +76,11 @@ export class AddonNotesListPage implements OnDestroy { } /** - * Fetch notes + * Fetch notes. + * * @param {boolean} sync When to resync notes. * @param {boolean} [showErrors] When to display errors or not. - * @return {Promise} Promise with the notes, + * @return {Promise} Promise with the notes. */ private fetchNotes(sync: boolean, showErrors?: boolean): Promise { const promise = sync ? this.syncNotes(showErrors) : Promise.resolve(); @@ -90,7 +91,7 @@ export class AddonNotesListPage implements OnDestroy { return this.notesProvider.getNotes(this.courseId).then((notes) => { notes = notes[this.type + 'notes'] || []; - this.hasOffline = this.notesProvider.hasOfflineNote(notes); + this.hasOffline = notes.some((note) => note.offline); return this.notesProvider.getNotesUserData(notes, this.courseId).then((notes) => { this.notes = notes; @@ -101,7 +102,7 @@ export class AddonNotesListPage implements OnDestroy { }).finally(() => { this.notesLoaded = true; this.refreshIcon = 'refresh'; - this.syncIcon = 'loop'; + this.syncIcon = 'sync'; }); } @@ -124,7 +125,7 @@ export class AddonNotesListPage implements OnDestroy { } /** - * Tries to syncrhonize course notes. + * Tries to synchronize course notes. * * @param {boolean} showErrors Whether to display errors or not. * @return {Promise} Promise resolved if sync is successful, rejected otherwise. diff --git a/src/addon/notes/providers/course-option-handler.ts b/src/addon/notes/providers/course-option-handler.ts index f71f9e097..8ab6dbbd1 100644 --- a/src/addon/notes/providers/course-option-handler.ts +++ b/src/addon/notes/providers/course-option-handler.ts @@ -25,24 +25,16 @@ import { AddonNotesTypesComponent } from '../components/types/types'; export class AddonNotesCourseOptionHandler implements CoreCourseOptionsHandler { name = 'AddonNotes'; priority = 200; - protected coursesNavEnabledCache = {}; constructor(private notesProvider: AddonNotesProvider) { } - /** - * Clear courses nav cache. - */ - clearCoursesNavCache(): void { - this.coursesNavEnabledCache = {}; - } - /** * Whether or not the handler is enabled on a site level. * @return {boolean|Promise} Whether or not the handler is enabled on a site level. */ isEnabled(): boolean | Promise { - return this.notesProvider.isPluginViewNotesEnabled(); + return this.notesProvider.isPluginEnabled(); } /** @@ -63,15 +55,7 @@ export class AddonNotesCourseOptionHandler implements CoreCourseOptionsHandler { return navOptions.notes; } - if (typeof this.coursesNavEnabledCache[courseId] != 'undefined') { - return this.coursesNavEnabledCache[courseId]; - } - - return this.notesProvider.isPluginViewNotesEnabledForCourse(courseId).then((enabled) => { - this.coursesNavEnabledCache[courseId] = enabled; - - return enabled; - }); + return this.notesProvider.isPluginViewNotesEnabledForCourse(courseId); } /** diff --git a/src/addon/notes/providers/notes.ts b/src/addon/notes/providers/notes.ts index 1f6e0199d..9e065ca01 100644 --- a/src/addon/notes/providers/notes.ts +++ b/src/addon/notes/providers/notes.ts @@ -32,7 +32,7 @@ export class AddonNotesProvider { protected logger; constructor(logger: CoreLoggerProvider, private sitesProvider: CoreSitesProvider, private appProvider: CoreAppProvider, - private utilsProvider: CoreUtilsProvider, private translate: TranslateService, private userProvider: CoreUserProvider, + private utils: CoreUtilsProvider, private translate: TranslateService, private userProvider: CoreUserProvider, private notesOffline: AddonNotesOfflineProvider) { this.logger = logger.getInstance('AddonNotesProvider'); } @@ -65,14 +65,14 @@ export class AddonNotesProvider { // Send note to server. return this.addNoteOnline(userId, courseId, publishState, noteText, siteId).then(() => { return true; - }).catch((data) => { - if (data.wserror) { - // It's a WebService error, the user cannot add the note so don't store it. - return Promise.reject(data.error); - } else { - // Error sending note, store it to retry later. - return storeOffline(); + }).catch((error) => { + if (this.utils.isWebServiceError(error)) { + // It's a WebService error, the user cannot send the message so don't store it. + return Promise.reject(error); } + + // Error sending note, store it to retry later. + return storeOffline(); }); } @@ -84,9 +84,7 @@ export class AddonNotesProvider { * @param {string} publishState Personal, Site or Course. * @param {string} noteText The note text. * @param {string} [siteId] Site ID. If not defined, current site. - * @return {Promise} Promise resolved when added, rejected otherwise. Reject param is an object with: - * - error: The error message. - * - wserror: True if it's an error returned by the WebService, false otherwise. + * @return {Promise} Promise resolved when added, rejected otherwise. */ addNoteOnline(userId: number, courseId: number, publishState: string, noteText: string, siteId?: string): Promise { const notes = [ @@ -99,18 +97,10 @@ export class AddonNotesProvider { } ]; - return this.addNotesOnline(notes, siteId).catch((error) => { - return Promise.reject({ - error: error, - wserror: this.utilsProvider.isWebServiceError(error) - }); - }).then((response) => { + return this.addNotesOnline(notes, siteId).then((response) => { if (response && response[0] && response[0].noteid === -1) { // There was an error, and it should be translated already. - return Promise.reject({ - error: response[0].errormessage, - wserror: true - }); + return Promise.reject(this.utils.createFakeWSError(response[0].errormessage)); } // A note was added, invalidate the course notes. @@ -143,7 +133,7 @@ export class AddonNotesProvider { } /** - * Returns whether or not the add note plugin is enabled for a certain site. + * Returns whether or not the notes plugin is enabled for a certain site. * * This method is called quite often and thus should only perform a quick * check, we should not be calling WS from here. @@ -151,15 +141,9 @@ export class AddonNotesProvider { * @param {string} [siteId] Site ID. If not defined, current site. * @return {Promise} Promise resolved with true if enabled, resolved with false or rejected otherwise. */ - isPluginAddNoteEnabled(siteId?: string): Promise { + isPluginEnabled(siteId?: string): Promise { return this.sitesProvider.getSite(siteId).then((site) => { - if (!site.canUseAdvancedFeature('enablenotes')) { - return false; - } else if (!site.wsAvailable('core_notes_create_notes')) { - return false; - } - - return true; + return site.canUseAdvancedFeature('enablenotes'); }); } @@ -188,33 +172,7 @@ export class AddonNotesProvider { /* Use .read to cache data and be able to check it in offline. This means that, if a user loses the capabilities to add notes, he'll still see the option in the app. */ - return site.read('core_notes_create_notes', data).then(() => { - // User can add notes. - return true; - }).catch(() => { - return false; - }); - }); - } - - /** - * Returns whether or not the read notes plugin is enabled for the current site. - * - * This method is called quite often and thus should only perform a quick - * check, we should not be calling WS from here. - * - * @param {string} [siteId] Site ID. If not defined, current site. - * @return {Promise} Promise resolved with true if enabled, resolved with false or rejected otherwise. - */ - isPluginViewNotesEnabled(siteId?: string): Promise { - return this.sitesProvider.getSite(siteId).then((site) => { - if (!site.canUseAdvancedFeature('enablenotes')) { - return false; - } else if (!site.wsAvailable('core_notes_get_course_notes')) { - return false; - } - - return true; + return this.utils.promiseWorks(site.read('core_notes_create_notes', data)); }); } @@ -226,11 +184,7 @@ export class AddonNotesProvider { * @return {Promise} Promise resolved with true if enabled, resolved with false or rejected otherwise. */ isPluginViewNotesEnabledForCourse(courseId: number, siteId?: string): Promise { - return this.getNotes(courseId, false, true, siteId).then(() => { - return true; - }).catch(() => { - return false; - }); + return this.utils.promiseWorks(this.getNotes(courseId, false, true, siteId)); } /** @@ -314,26 +268,6 @@ export class AddonNotesProvider { }); } - /** - * Given a list of notes, check if any of them is an offline note. - * - * @param {any[]} notes List of notes. - * @return {boolean} True if at least 1 note is offline, false otherwise. - */ - hasOfflineNote(notes: any[]): boolean { - if (!notes || !notes.length) { - return false; - } - - for (let i = 0, len = notes.length; i < len; i++) { - if (notes[i].offline) { - return true; - } - } - - return false; - } - /** * Invalidate get notes WS call. * diff --git a/src/addon/notes/providers/user-handler.ts b/src/addon/notes/providers/user-handler.ts index 97478efe1..ad48c0f06 100644 --- a/src/addon/notes/providers/user-handler.ts +++ b/src/addon/notes/providers/user-handler.ts @@ -14,6 +14,8 @@ import { Injectable } from '@angular/core'; import { ModalController } from 'ionic-angular'; +import { CoreEventsProvider } from '@providers/events'; +import { CoreUserProvider } from '@core/user/providers/user'; import { CoreUserDelegate, CoreUserProfileHandler, CoreUserProfileHandlerData } from '@core/user/providers/user-delegate'; import { CoreSitesProvider } from '@providers/sites'; import { AddonNotesProvider } from './notes'; @@ -29,7 +31,11 @@ export class AddonNotesUserHandler implements CoreUserProfileHandler { addNoteEnabledCache = {}; constructor(private modalCtrl: ModalController, private sitesProvider: CoreSitesProvider, - private notesProvider: AddonNotesProvider) { + private notesProvider: AddonNotesProvider, eventsProvider: CoreEventsProvider) { + eventsProvider.on(CoreEventsProvider.LOGOUT, this.clearAddNoteCache.bind(this)); + eventsProvider.on(CoreUserProvider.PROFILE_REFRESHED, (data) => { + this.clearAddNoteCache(data.courseId); + }); } /** @@ -38,7 +44,7 @@ export class AddonNotesUserHandler implements CoreUserProfileHandler { * * @param {number} [courseId] Course ID. */ - clearAddNoteCache(courseId?: number): void { + private clearAddNoteCache(courseId?: number): void { if (courseId) { delete this.addNoteEnabledCache[courseId]; } else { @@ -51,7 +57,7 @@ export class AddonNotesUserHandler implements CoreUserProfileHandler { * @return {boolean|Promise} Whether or not the handler is enabled on a site level. */ isEnabled(): boolean | Promise { - return this.notesProvider.isPluginAddNoteEnabled(); + return this.notesProvider.isPluginEnabled(); } /**