From a5fbe051dac07a99e05ab6afb7042643ec96b99c Mon Sep 17 00:00:00 2001 From: Dani Palou Date: Thu, 16 Jun 2022 15:08:17 +0200 Subject: [PATCH 01/14] MOBILE-3817 core: Split CoreSite.request into several functions --- src/core/classes/site.ts | 368 +++++++++++++++++++++++---------------- 1 file changed, 219 insertions(+), 149 deletions(-) diff --git a/src/core/classes/site.ts b/src/core/classes/site.ts index cdd509929..b54787346 100644 --- a/src/core/classes/site.ts +++ b/src/core/classes/site.ts @@ -536,7 +536,6 @@ export class CoreSite { * @param method The WebService method to be called. * @param data Arguments to pass to the method. * @param preSets Extra options. - * @param retrying True if we're retrying the call for some reason. This is to prevent infinite loops. * @return Promise resolved with the response, rejected with CoreWSError if it fails. * @description * @@ -547,7 +546,7 @@ export class CoreSite { * data hasn't expired. */ // eslint-disable-next-line @typescript-eslint/no-explicit-any - async request(method: string, data: any, preSets: CoreSiteWSPreSets, retrying?: boolean): Promise { + async request(method: string, data: any, preSets: CoreSiteWSPreSets): Promise { if (this.isLoggedOut() && !ALLOWED_LOGGEDOUT_WS.includes(method)) { // Site is logged out, it cannot call WebServices. CoreEvents.trigger(CoreEvents.SESSION_EXPIRED, {}, this.id); @@ -556,7 +555,6 @@ export class CoreSite { throw new CoreSilentError(Translate.instant('core.lostconnection')); } - const initialToken = this.token || ''; data = data || {}; if (!CoreNetwork.isOnline() && this.offlineDisabled) { @@ -616,152 +614,7 @@ export class CoreSite { return CoreUtils.clone(response); } - const promise = this.getFromCache(method, data, preSets, false).catch(async () => { - if (preSets.forceOffline) { - // Don't call the WS, just fail. - throw new CoreError( - Translate.instant('core.cannotconnect', { $a: CoreSite.MINIMUM_MOODLE_VERSION }), - ); - } - - // Call the WS. - try { - if (method !== 'core_webservice_get_site_info') { - // Send the language to use. Do it after checking cache to prevent losing offline data when changing language. - // Don't send it to core_webservice_get_site_info, that WS is used to check if Moodle version is supported. - data.moodlewssettinglang = preSets.lang ?? await CoreLang.getCurrentLanguage(); - // Moodle uses underscore instead of dash. - data.moodlewssettinglang = data.moodlewssettinglang.replace('-', '_'); - } - - const response = await this.callOrEnqueueRequest(method, data, preSets, wsPreSets); - - if (preSets.saveToCache) { - delete data.moodlewssettinglang; - this.saveToCache(method, data, response, preSets); - } - - return response; - } catch (error) { - let useSilentError = false; - - if (CoreUtils.isExpiredTokenError(error)) { - if (initialToken !== this.token && !retrying) { - // Token has changed, retry with the new token. - preSets.getFromCache = false; // Don't check cache now. Also, it will skip ongoingRequests. - - return this.request(method, data, preSets, true); - } else if (CoreApp.isSSOAuthenticationOngoing()) { - // There's an SSO authentication ongoing, wait for it to finish and try again. - await CoreApp.waitForSSOAuthentication(); - - return this.request(method, data, preSets, true); - } - - // Session expired, trigger event. - CoreEvents.trigger(CoreEvents.SESSION_EXPIRED, {}, this.id); - // Change error message. Try to get data from cache, the event will handle the error. - error.message = Translate.instant('core.lostconnection'); - useSilentError = true; // Use a silent error, the SESSION_EXPIRED event will display a message if needed. - } else if (error.errorcode === 'userdeleted' || error.errorcode === 'wsaccessuserdeleted') { - // User deleted, trigger event. - CoreEvents.trigger(CoreEvents.USER_DELETED, { params: data }, this.id); - error.message = Translate.instant('core.userdeleted'); - - throw new CoreWSError(error); - } else if (error.errorcode === 'wsaccessusersuspended') { - // User suspended, trigger event. - CoreEvents.trigger(CoreEvents.USER_SUSPENDED, { params: data }, this.id); - error.message = Translate.instant('core.usersuspended'); - - throw new CoreWSError(error); - } else if (error.errorcode === 'wsaccessusernologin') { - // User suspended, trigger event. - CoreEvents.trigger(CoreEvents.USER_NO_LOGIN, { params: data }, this.id); - error.message = Translate.instant('core.usernologin'); - - throw new CoreWSError(error); - } else if (error.errorcode === 'forcepasswordchangenotice') { - // Password Change Forced, trigger event. Try to get data from cache, the event will handle the error. - CoreEvents.trigger(CoreEvents.PASSWORD_CHANGE_FORCED, {}, this.id); - error.message = Translate.instant('core.forcepasswordchangenotice'); - useSilentError = true; // Use a silent error, the change password page already displays the appropiate info. - } else if (error.errorcode === 'usernotfullysetup') { - // User not fully setup, trigger event. Try to get data from cache, the event will handle the error. - CoreEvents.trigger(CoreEvents.USER_NOT_FULLY_SETUP, {}, this.id); - error.message = Translate.instant('core.usernotfullysetup'); - useSilentError = true; // Use a silent error, the complete profile page already displays the appropiate info. - } else if (error.errorcode === 'sitepolicynotagreed') { - // Site policy not agreed, trigger event. - CoreEvents.trigger(CoreEvents.SITE_POLICY_NOT_AGREED, {}, this.id); - error.message = Translate.instant('core.login.sitepolicynotagreederror'); - - throw new CoreWSError(error); - } else if (error.errorcode === 'dmlwriteexception' && CoreTextUtils.hasUnicodeData(data)) { - if (!this.cleanUnicode) { - // Try again cleaning unicode. - this.cleanUnicode = true; - - return this.request(method, data, preSets); - } - // This should not happen. - error.message = Translate.instant('core.unicodenotsupported'); - - throw new CoreWSError(error); - } else if (error.exception === 'required_capability_exception' || error.errorcode === 'nopermission' || - error.errorcode === 'notingroup') { - // Translate error messages with missing strings. - if (error.message === 'error/nopermission') { - error.message = Translate.instant('core.nopermissionerror'); - } else if (error.message === 'error/notingroup') { - error.message = Translate.instant('core.notingroup'); - } - - // Save the error instead of deleting the cache entry so the same content is displayed in offline. - this.saveToCache(method, data, error, preSets); - - throw new CoreWSError(error); - } else if (preSets.cacheErrors && preSets.cacheErrors.indexOf(error.errorcode) != -1) { - // Save the error instead of deleting the cache entry so the same content is displayed in offline. - this.saveToCache(method, data, error, preSets); - - throw new CoreWSError(error); - } else if (preSets.emergencyCache !== undefined && !preSets.emergencyCache) { - this.logger.debug(`WS call '${method}' failed. Emergency cache is forbidden, rejecting.`); - - throw new CoreWSError(error); - } - - if (preSets.deleteCacheIfWSError && CoreUtils.isWebServiceError(error)) { - // Delete the cache entry and return the entry. Don't block the user with the delete. - CoreUtils.ignoreErrors(this.deleteFromCache(method, data, preSets)); - - throw new CoreWSError(error); - } - - this.logger.debug(`WS call '${method}' failed. Trying to use the emergency cache.`); - preSets.omitExpires = true; - preSets.getFromCache = true; - - try { - return await this.getFromCache(method, data, preSets, true); - } catch { - if (useSilentError) { - throw new CoreSilentError(error.message); - } - - throw new CoreWSError(error); - } - } - // eslint-disable-next-line @typescript-eslint/no-explicit-any - }).then((response: any) => { - // Check if the response is an error, this happens if the error was stored in the cache. - if (response && (response.exception !== undefined || response.errorcode !== undefined)) { - throw new CoreWSError(response); - } - - return response; - }); + const promise = this.performRequest(method, data, preSets, wsPreSets); this.ongoingRequests[cacheId] = promise; @@ -779,6 +632,223 @@ export class CoreSite { } } + /** + * Perform a request, getting the response either from cache or WebService. + * + * @param method The WebService method to be called. + * @param data Arguments to pass to the method. + * @param preSets Extra options related to the site. + * @param wsPreSets Extra options related to the WS call. + * @return Promise resolved with the response. + */ + protected async performRequest( + method: string, + data: unknown, + preSets: CoreSiteWSPreSets, + wsPreSets: CoreWSPreSets, + ): Promise { + let response: T | {exception?: string; errorcode?: string}; + + try { + response = await this.getFromCache(method, data, preSets, false); + } catch { + // Not found or expired, call WS. + response = await this.getFromWSOrEmergencyCache(method, data, preSets, wsPreSets); + } + + if (('exception' in response && response.exception !== undefined) || + ('errorcode' in response && response.errorcode !== undefined)) { + throw new CoreWSError(response); + } + + return response; + } + + /** + * Get a request response from WS, if it fails it might try to get it from emergency cache. + * + * @param method The WebService method to be called. + * @param data Arguments to pass to the method. + * @param preSets Extra options related to the site. + * @param wsPreSets Extra options related to the WS call. + * @return Promise resolved with the response. + */ + protected async getFromWSOrEmergencyCache( + method: string, + data: any, // eslint-disable-line @typescript-eslint/no-explicit-any + preSets: CoreSiteWSPreSets, + wsPreSets: CoreWSPreSets, + ): Promise { + if (preSets.forceOffline) { + // Don't call the WS, just fail. + throw new CoreError( + Translate.instant('core.cannotconnect', { $a: CoreSite.MINIMUM_MOODLE_VERSION }), + ); + } + + try { + const response = await this.getFromWS(method, data, preSets, wsPreSets); + + if (preSets.saveToCache) { + this.saveToCache(method, data, response, preSets); + } + + return response; + } catch (error) { + let useSilentError = false; + + if (CoreUtils.isExpiredTokenError(error)) { + // Session expired, trigger event. + CoreEvents.trigger(CoreEvents.SESSION_EXPIRED, {}, this.id); + // Change error message. Try to get data from cache, the event will handle the error. + error.message = Translate.instant('core.lostconnection'); + useSilentError = true; // Use a silent error, the SESSION_EXPIRED event will display a message if needed. + } else if (error.errorcode === 'userdeleted' || error.errorcode === 'wsaccessuserdeleted') { + // User deleted, trigger event. + CoreEvents.trigger(CoreEvents.USER_DELETED, { params: data }, this.id); + error.message = Translate.instant('core.userdeleted'); + + throw new CoreWSError(error); + } else if (error.errorcode === 'wsaccessusersuspended') { + // User suspended, trigger event. + CoreEvents.trigger(CoreEvents.USER_SUSPENDED, { params: data }, this.id); + error.message = Translate.instant('core.usersuspended'); + + throw new CoreWSError(error); + } else if (error.errorcode === 'wsaccessusernologin') { + // User suspended, trigger event. + CoreEvents.trigger(CoreEvents.USER_NO_LOGIN, { params: data }, this.id); + error.message = Translate.instant('core.usernologin'); + + throw new CoreWSError(error); + } else if (error.errorcode === 'forcepasswordchangenotice') { + // Password Change Forced, trigger event. Try to get data from cache, the event will handle the error. + CoreEvents.trigger(CoreEvents.PASSWORD_CHANGE_FORCED, {}, this.id); + error.message = Translate.instant('core.forcepasswordchangenotice'); + useSilentError = true; // Use a silent error, the change password page already displays the appropiate info. + } else if (error.errorcode === 'usernotfullysetup') { + // User not fully setup, trigger event. Try to get data from cache, the event will handle the error. + CoreEvents.trigger(CoreEvents.USER_NOT_FULLY_SETUP, {}, this.id); + error.message = Translate.instant('core.usernotfullysetup'); + useSilentError = true; // Use a silent error, the complete profile page already displays the appropiate info. + } else if (error.errorcode === 'sitepolicynotagreed') { + // Site policy not agreed, trigger event. + CoreEvents.trigger(CoreEvents.SITE_POLICY_NOT_AGREED, {}, this.id); + error.message = Translate.instant('core.login.sitepolicynotagreederror'); + + throw new CoreWSError(error); + } else if (error.errorcode === 'dmlwriteexception' && CoreTextUtils.hasUnicodeData(data)) { + if (!this.cleanUnicode) { + // Try again cleaning unicode. + this.cleanUnicode = true; + + return this.request(method, data, preSets); + } + // This should not happen. + error.message = Translate.instant('core.unicodenotsupported'); + + throw new CoreWSError(error); + } else if (error.exception === 'required_capability_exception' || error.errorcode === 'nopermission' || + error.errorcode === 'notingroup') { + // Translate error messages with missing strings. + if (error.message === 'error/nopermission') { + error.message = Translate.instant('core.nopermissionerror'); + } else if (error.message === 'error/notingroup') { + error.message = Translate.instant('core.notingroup'); + } + + if (preSets.saveToCache) { + // Save the error instead of deleting the cache entry so the same content is displayed in offline. + this.saveToCache(method, data, error, preSets); + } + + throw new CoreWSError(error); + } else if (preSets.cacheErrors && preSets.cacheErrors.indexOf(error.errorcode) != -1) { + // Save the error instead of deleting the cache entry so the same content is displayed in offline. + this.saveToCache(method, data, error, preSets); + + throw new CoreWSError(error); + } else if (preSets.emergencyCache !== undefined && !preSets.emergencyCache) { + this.logger.debug(`WS call '${method}' failed. Emergency cache is forbidden, rejecting.`); + + throw new CoreWSError(error); + } + + if (preSets.deleteCacheIfWSError && CoreUtils.isWebServiceError(error)) { + // Delete the cache entry and return the entry. Don't block the user with the delete. + CoreUtils.ignoreErrors(this.deleteFromCache(method, data, preSets)); + + throw new CoreWSError(error); + } + + this.logger.debug(`WS call '${method}' failed. Trying to use the emergency cache.`); + preSets.omitExpires = true; + preSets.getFromCache = true; + + try { + return await this.getFromCache(method, data, preSets, true); + } catch { + if (useSilentError) { + throw new CoreSilentError(error.message); + } + + throw new CoreWSError(error); + } + } + } + + /** + * Get a request response from WS. + * + * @param method The WebService method to be called. + * @param data Arguments to pass to the method. + * @param preSets Extra options related to the site. + * @param wsPreSets Extra options related to the WS call. + * @return Promise resolved with the response. + */ + protected async getFromWS( + method: string, + data: any, // eslint-disable-line @typescript-eslint/no-explicit-any + preSets: CoreSiteWSPreSets, + wsPreSets: CoreWSPreSets, + ): Promise { + // Call the WS. + const initialToken = this.token ?? ''; + + // Call the WS. + if (method !== 'core_webservice_get_site_info') { + // Send the language to use. Do it after checking cache to prevent losing offline data when changing language. + // Don't send it to core_webservice_get_site_info, that WS is used to check if Moodle version is supported. + data = { + ...data, + moodlewssettinglang: preSets.lang ?? await CoreLang.getCurrentLanguage(), + }; + // Moodle uses underscore instead of dash. + data.moodlewssettinglang = data.moodlewssettinglang.replace('-', '_'); + + } + + try { + return await this.callOrEnqueueRequest(method, data, preSets, wsPreSets); + } catch (error) { + if (CoreUtils.isExpiredTokenError(error)) { + if (initialToken !== this.token) { + // Token has changed, retry with the new token. + wsPreSets.wsToken = this.token ?? ''; + + return await this.callOrEnqueueRequest(method, data, preSets, wsPreSets); + } else if (CoreApp.isSSOAuthenticationOngoing()) { + // There's an SSO authentication ongoing, wait for it to finish and try again. + await CoreApp.waitForSSOAuthentication(); + + return await this.callOrEnqueueRequest(method, data, preSets, wsPreSets); + } + } + + throw error; + } + } + /** * Adds a request to the queue or calls it immediately when not using the queue. * From f41a4e7b572e5f7c224b018c1aa1b4e11651ee1e Mon Sep 17 00:00:00 2001 From: Dani Palou Date: Tue, 21 Jun 2022 10:58:10 +0200 Subject: [PATCH 02/14] MOBILE-3817 core: Create observable methods for WS requests --- src/core/classes/site.ts | 194 ++++++++++++++++++++++++--------------- 1 file changed, 119 insertions(+), 75 deletions(-) diff --git a/src/core/classes/site.ts b/src/core/classes/site.ts index b54787346..7dd12b4e5 100644 --- a/src/core/classes/site.ts +++ b/src/core/classes/site.ts @@ -57,6 +57,8 @@ import { WSGroups, WS_CACHE_TABLES_PREFIX, } from '@services/database/sites'; +import { Observable, Subject } from 'rxjs'; +import { finalize, map } from 'rxjs/operators'; /** * QR Code type enumeration. @@ -122,7 +124,7 @@ export class CoreSite { protected lastAutoLogin = 0; protected offlineDisabled = false; // eslint-disable-next-line @typescript-eslint/no-explicit-any - protected ongoingRequests: { [cacheId: string]: Promise } = {}; + protected ongoingRequests: { [cacheId: string]: Observable } = {}; protected requestQueue: RequestQueueItem[] = []; protected requestQueueTimeout: number | null = null; protected tokenPluginFileWorks?: boolean; @@ -492,18 +494,25 @@ export class CoreSite { */ // eslint-disable-next-line @typescript-eslint/no-explicit-any read(method: string, data: any, preSets?: CoreSiteWSPreSets): Promise { - preSets = preSets || {}; - if (preSets.getFromCache === undefined) { - preSets.getFromCache = true; - } - if (preSets.saveToCache === undefined) { - preSets.saveToCache = true; - } - if (preSets.reusePending === undefined) { - preSets.reusePending = true; - } + return this.readObservable(method, data, preSets).toPromise(); + } - return this.request(method, data, preSets); + /** + * Read some data from the Moodle site using WS. Requests are cached by default. + * + * @param method WS method to use. + * @param data Data to send to the WS. + * @param preSets Extra options. + * @return Observable. + */ + // eslint-disable-next-line @typescript-eslint/no-explicit-any + readObservable(method: string, data: any, preSets?: CoreSiteWSPreSets): Observable { + preSets = preSets || {}; + preSets.getFromCache = preSets.getFromCache ?? true; + preSets.saveToCache = preSets.saveToCache ?? true; + preSets.reusePending = preSets.reusePending ?? true; + + return this.requestObservable(method, data, preSets); } /** @@ -516,18 +525,25 @@ export class CoreSite { */ // eslint-disable-next-line @typescript-eslint/no-explicit-any write(method: string, data: any, preSets?: CoreSiteWSPreSets): Promise { - preSets = preSets || {}; - if (preSets.getFromCache === undefined) { - preSets.getFromCache = false; - } - if (preSets.saveToCache === undefined) { - preSets.saveToCache = false; - } - if (preSets.emergencyCache === undefined) { - preSets.emergencyCache = false; - } + return this.writeObservable(method, data, preSets).toPromise(); + } - return this.request(method, data, preSets); + /** + * Sends some data to the Moodle site using WS. Requests are NOT cached by default. + * + * @param method WS method to use. + * @param data Data to send to the WS. + * @param preSets Extra options. + * @return Observable. + */ + // eslint-disable-next-line @typescript-eslint/no-explicit-any + writeObservable(method: string, data: any, preSets?: CoreSiteWSPreSets): Observable { + preSets = preSets || {}; + preSets.getFromCache = preSets.getFromCache ?? false; + preSets.saveToCache = preSets.saveToCache ?? false; + preSets.emergencyCache = preSets.emergencyCache ?? false; + + return this.requestObservable(method, data, preSets); } /** @@ -537,6 +553,19 @@ export class CoreSite { * @param data Arguments to pass to the method. * @param preSets Extra options. * @return Promise resolved with the response, rejected with CoreWSError if it fails. + */ + // eslint-disable-next-line @typescript-eslint/no-explicit-any + async request(method: string, data: any, preSets: CoreSiteWSPreSets): Promise { + return this.requestObservable(method, data, preSets).toPromise(); + } + + /** + * WS request to the site. + * + * @param method The WebService method to be called. + * @param data Arguments to pass to the method. + * @param preSets Extra options. + * @return Observable * @description * * Sends a webservice request to the site. This method will automatically add the @@ -546,7 +575,7 @@ export class CoreSite { * data hasn't expired. */ // eslint-disable-next-line @typescript-eslint/no-explicit-any - async request(method: string, data: any, preSets: CoreSiteWSPreSets): Promise { + requestObservable(method: string, data: any, preSets: CoreSiteWSPreSets): Observable { if (this.isLoggedOut() && !ALLOWED_LOGGEDOUT_WS.includes(method)) { // Site is logged out, it cannot call WebServices. CoreEvents.trigger(CoreEvents.SESSION_EXPIRED, {}, this.id); @@ -608,28 +637,24 @@ export class CoreSite { // Check for an ongoing identical request if we're not ignoring cache. if (preSets.getFromCache && this.ongoingRequests[cacheId] !== undefined) { - const response = await this.ongoingRequests[cacheId]; - - // Clone the data, this may prevent errors if in the callback the object is modified. - return CoreUtils.clone(response); + return this.ongoingRequests[cacheId]; } - const promise = this.performRequest(method, data, preSets, wsPreSets); + const observable = this.performRequest(method, data, preSets, wsPreSets).pipe( + // Return a clone of the original object, this may prevent errors if in the callback the object is modified. + map((data) => CoreUtils.clone(data)), + ); - this.ongoingRequests[cacheId] = promise; + this.ongoingRequests[cacheId] = observable; - // Clear ongoing request after setting the promise (just in case it's already resolved). - try { - const response = await promise; - - // We pass back a clone of the original object, this may prevent errors if in the callback the object is modified. - return CoreUtils.clone(response); - } finally { - // Make sure we don't clear the promise of a newer request that ignores the cache. - if (this.ongoingRequests[cacheId] === promise) { - delete this.ongoingRequests[cacheId]; - } - } + return observable.pipe( + finalize(() => { + // Clear the ongoing request unless it has changed (e.g. a new request that ignores cache). + if (this.ongoingRequests[cacheId] === observable) { + delete this.ongoingRequests[cacheId]; + } + }), + ); } /** @@ -639,29 +664,42 @@ export class CoreSite { * @param data Arguments to pass to the method. * @param preSets Extra options related to the site. * @param wsPreSets Extra options related to the WS call. - * @return Promise resolved with the response. + * @return Observable. */ - protected async performRequest( + protected performRequest( method: string, data: unknown, preSets: CoreSiteWSPreSets, wsPreSets: CoreWSPreSets, - ): Promise { - let response: T | {exception?: string; errorcode?: string}; + ): Observable { + const subject = new Subject(); - try { - response = await this.getFromCache(method, data, preSets, false); - } catch { - // Not found or expired, call WS. - response = await this.getFromWSOrEmergencyCache(method, data, preSets, wsPreSets); - } + const run = async () => { + try { + let response: T | {exception?: string; errorcode?: string}; - if (('exception' in response && response.exception !== undefined) || - ('errorcode' in response && response.errorcode !== undefined)) { - throw new CoreWSError(response); - } + try { + response = await this.getFromCache(method, data, preSets, false); + } catch { + // Not found or expired, call WS. + response = await this.getFromWSOrEmergencyCache(method, data, preSets, wsPreSets); + } - return response; + if (('exception' in response && response.exception !== undefined) || + ('errorcode' in response && response.errorcode !== undefined)) { + throw new CoreWSError(response); + } + + subject.next( response); + subject.complete(); + } catch (error) { + subject.error(error); + } + }; + + run(); + + return subject; } /** @@ -1536,12 +1574,24 @@ export class CoreSite { // Check for an ongoing identical request if we're not ignoring cache. if (cachePreSets.getFromCache && this.ongoingRequests[cacheId] !== undefined) { - const response = await this.ongoingRequests[cacheId]; - - return response; + return await this.ongoingRequests[cacheId].toPromise(); } - const promise = this.getFromCache(method, {}, cachePreSets, false).catch(async () => { + const subject = new Subject(); + const observable = subject.pipe( + // Return a clone of the original object, this may prevent errors if in the callback the object is modified. + map((data) => CoreUtils.clone(data)), + finalize(() => { + // Clear the ongoing request unless it has changed (e.g. a new request that ignores cache). + if (this.ongoingRequests[cacheId] === observable) { + delete this.ongoingRequests[cacheId]; + } + }), + ); + + this.ongoingRequests[cacheId] = observable; + + this.getFromCache(method, {}, cachePreSets, false).catch(async () => { if (cachePreSets.forceOffline) { // Don't call the WS, just fail. throw new CoreError( @@ -1568,22 +1618,16 @@ export class CoreSite { throw error; } } + }).then((response) => { + subject.next(response); + subject.complete(); + + return; + }).catch((error) => { + subject.error(error); }); - this.ongoingRequests[cacheId] = promise; - - // Clear ongoing request after setting the promise (just in case it's already resolved). - try { - const response = await promise; - - // We pass back a clone of the original object, this may prevent errors if in the callback the object is modified. - return response; - } finally { - // Make sure we don't clear the promise of a newer request that ignores the cache. - if (this.ongoingRequests[cacheId] === promise) { - delete this.ongoingRequests[cacheId]; - } - } + return observable.toPromise(); } /** From 89ba05dd3e1e023ea97ca603f4eb030311c00725 Mon Sep 17 00:00:00 2001 From: Dani Palou Date: Tue, 21 Jun 2022 14:50:08 +0200 Subject: [PATCH 03/14] MOBILE-3817 core: Support updating WS data in background --- src/core/classes/site.ts | 204 +++++++++++++++++++++++++++---------- src/core/services/sites.ts | 7 ++ src/types/config.d.ts | 2 + 3 files changed, 162 insertions(+), 51 deletions(-) diff --git a/src/core/classes/site.ts b/src/core/classes/site.ts index 7dd12b4e5..0fbc8c42d 100644 --- a/src/core/classes/site.ts +++ b/src/core/classes/site.ts @@ -676,22 +676,59 @@ export class CoreSite { const run = async () => { try { - let response: T | {exception?: string; errorcode?: string}; + let response: T | WSCachedError; + let cachedData: WSCachedData | undefined; try { - response = await this.getFromCache(method, data, preSets, false); + cachedData = await this.getFromCache(method, data, preSets, false); + response = cachedData.response; } catch { // Not found or expired, call WS. - response = await this.getFromWSOrEmergencyCache(method, data, preSets, wsPreSets); + response = await this.getFromWS(method, data, preSets, wsPreSets); } - if (('exception' in response && response.exception !== undefined) || - ('errorcode' in response && response.errorcode !== undefined)) { - throw new CoreWSError(response); + if ( + typeof response === 'object' && response !== null && + ( + ('exception' in response && response.exception !== undefined) || + ('errorcode' in response && response.errorcode !== undefined) + ) + ) { + subject.error(new CoreWSError(response)); + } else { + subject.next( response); } - subject.next( response); - subject.complete(); + if ( + preSets.updateInBackground && + !CoreConstants.CONFIG.disableCallWSInBackground && + cachedData && + !cachedData.expirationIgnored && + cachedData.expirationTime !== undefined && + Date.now() > cachedData.expirationTime + ) { + // Update the data in background. + setTimeout(async () => { + try { + preSets = { + ...preSets, + emergencyCache: false, + }; + + const newData = await this.getFromWS(method, data, preSets, wsPreSets); + + subject.next(newData); + } catch (error) { + // Ignore errors when updating in background. + this.logger.error('Error updating WS data in background', error); + } finally { + subject.complete(); + } + }); + } else { + // No need to update in background, complete the observable. + subject.complete(); + } } catch (error) { subject.error(error); } @@ -711,7 +748,7 @@ export class CoreSite { * @param wsPreSets Extra options related to the WS call. * @return Promise resolved with the response. */ - protected async getFromWSOrEmergencyCache( + protected async getFromWS( method: string, data: any, // eslint-disable-line @typescript-eslint/no-explicit-any preSets: CoreSiteWSPreSets, @@ -725,7 +762,7 @@ export class CoreSite { } try { - const response = await this.getFromWS(method, data, preSets, wsPreSets); + const response = await this.callOrEnqueueWS(method, data, preSets, wsPreSets); if (preSets.saveToCache) { this.saveToCache(method, data, response, preSets); @@ -820,11 +857,26 @@ export class CoreSite { } this.logger.debug(`WS call '${method}' failed. Trying to use the emergency cache.`); - preSets.omitExpires = true; - preSets.getFromCache = true; + preSets = { + ...preSets, + omitExpires: true, + getFromCache: true, + }; try { - return await this.getFromCache(method, data, preSets, true); + const cachedData = await this.getFromCache(method, data, preSets, true); + + if ( + typeof cachedData.response === 'object' && cachedData.response !== null && + ( + ('exception' in cachedData.response && cachedData.response.exception !== undefined) || + ('errorcode' in cachedData.response && cachedData.response.errorcode !== undefined) + ) + ) { + throw new CoreWSError(cachedData.response); + } + + return cachedData.response; } catch { if (useSilentError) { throw new CoreSilentError(error.message); @@ -844,7 +896,7 @@ export class CoreSite { * @param wsPreSets Extra options related to the WS call. * @return Promise resolved with the response. */ - protected async getFromWS( + protected async callOrEnqueueWS( method: string, data: any, // eslint-disable-line @typescript-eslint/no-explicit-any preSets: CoreSiteWSPreSets, @@ -1085,14 +1137,14 @@ export class CoreSite { * @param preSets Extra options. * @param emergency Whether it's an "emergency" cache call (WS call failed). * @param originalData Arguments to pass to the method before being converted to strings. - * @return Promise resolved with the WS response. + * @return Cached data. */ protected async getFromCache( method: string, data: any, // eslint-disable-line @typescript-eslint/no-explicit-any preSets: CoreSiteWSPreSets, emergency?: boolean, - ): Promise { + ): Promise> { if (!this.db || !preSets.getFromCache) { throw new CoreError('Get from cache is disabled.'); } @@ -1128,12 +1180,22 @@ export class CoreSite { const now = Date.now(); let expirationTime: number | undefined; - preSets.omitExpires = preSets.omitExpires || preSets.forceOffline || !CoreNetwork.isOnline(); + const forceCache = preSets.omitExpires || preSets.forceOffline || !CoreNetwork.isOnline(); - if (!preSets.omitExpires) { + if (!forceCache) { expirationTime = entry.expirationTime + this.getExpirationDelay(preSets.updateFrequency); - if (now > expirationTime) { + if (preSets.updateInBackground && !CoreConstants.CONFIG.disableCallWSInBackground) { + // Use a extended expiration time. + const extendedTime = entry.expirationTime + + (CoreConstants.CONFIG.callWSInBackgroundExpirationTime ?? CoreConstants.SECONDS_WEEK * 1000); + + if (now > extendedTime) { + this.logger.debug('Cached element found, but it is expired even for call WS in background.'); + + throw new CoreError('Cache entry is expired.'); + } + } else if (now > expirationTime) { this.logger.debug('Cached element found, but it is expired'); throw new CoreError('Cache entry is expired.'); @@ -1148,7 +1210,11 @@ export class CoreSite { this.logger.info(`Cached element found, id: ${id}. Expires in expires in ${expires} seconds`); } - return CoreTextUtils.parseJSON(entry.data, {}); + return { + response: CoreTextUtils.parseJSON(entry.data, {}), + expirationIgnored: forceCache, + expirationTime, + }; } throw new CoreError('Cache entry not valid.'); @@ -1591,41 +1657,46 @@ export class CoreSite { this.ongoingRequests[cacheId] = observable; - this.getFromCache(method, {}, cachePreSets, false).catch(async () => { - if (cachePreSets.forceOffline) { - // Don't call the WS, just fail. - throw new CoreError( - Translate.instant('core.cannotconnect', { $a: CoreSite.MINIMUM_MOODLE_VERSION }), - ); - } - - // Call the WS. - try { - const config = await this.requestPublicConfig(); - - if (cachePreSets.saveToCache) { - this.saveToCache(method, {}, config, cachePreSets); + this.getFromCache(method, {}, cachePreSets, false) + .then(cachedData => cachedData.response) + .catch(async () => { + if (cachePreSets.forceOffline) { + // Don't call the WS, just fail. + throw new CoreError( + Translate.instant('core.cannotconnect', { $a: CoreSite.MINIMUM_MOODLE_VERSION }), + ); } - return config; - } catch (error) { - cachePreSets.omitExpires = true; - cachePreSets.getFromCache = true; - + // Call the WS. try { - return await this.getFromCache(method, {}, cachePreSets, true); - } catch { - throw error; - } - } - }).then((response) => { - subject.next(response); - subject.complete(); + const config = await this.requestPublicConfig(); - return; - }).catch((error) => { - subject.error(error); - }); + if (cachePreSets.saveToCache) { + this.saveToCache(method, {}, config, cachePreSets); + } + + return config; + } catch (error) { + cachePreSets.omitExpires = true; + cachePreSets.getFromCache = true; + + try { + const cachedData = await this.getFromCache(method, {}, cachePreSets, true); + + return cachedData.response; + } catch { + throw error; + } + } + }).then((response) => { + // The app doesn't store exceptions for this call, it's safe to assume type CoreSitePublicConfigResponse. + subject.next( response); + subject.complete(); + + return; + }).catch((error) => { + subject.error(error); + }); return observable.toPromise(); } @@ -2425,6 +2496,12 @@ export type CoreSiteWSPreSets = { * can cause the request to fail (see PHP's max_input_vars). */ splitRequest?: CoreWSPreSetsSplitRequest; + + /** + * If true, the app will return cached data even if it's expired and then it'll call the WS in the background. + * Only enabled if CoreConstants.CONFIG.disableCallWSInBackground isn't true. + */ + updateInBackground?: boolean; }; /** @@ -2625,3 +2702,28 @@ export type CoreSiteStoreLastViewedOptions = { data?: string; // Other data. timeaccess?: number; // Accessed time. If not set, current time. }; + +/** + * Info about cached data. + */ +type WSCachedData = { + response: T | WSCachedError; // The WS response data, or an error if the WS returned an error and it was cached. + expirationIgnored: boolean; // Whether the expiration time was ignored. + expirationTime?: number; // Entry expiration time (only if not ignored). +}; + +/** + * Error data stored in cache. + */ +type WSCachedError = { + exception?: string; + errorcode?: string; +}; + +/** + * Observable returned when calling WebServices. + * If the request uses the "update in background" feature, it will return 2 values: first the cached one, and then the one + * coming from the server. After this, it will complete. + * Otherwise, it will only return 1 value, either coming from cache or from the server. After this, it will complete. + */ +export type WSObservable = Observable; diff --git a/src/core/services/sites.ts b/src/core/services/sites.ts index 67754f37b..c16638ac3 100644 --- a/src/core/services/sites.ts +++ b/src/core/services/sites.ts @@ -1796,6 +1796,12 @@ export class CoreSitesProvider { getFromCache: false, emergencyCache: false, }; + case CoreSitesReadingStrategy.UPDATE_IN_BACKGROUND: + return { + updateInBackground: true, + getFromCache: true, + saveToCache: true, + }; default: return {}; } @@ -2017,6 +2023,7 @@ export const enum CoreSitesReadingStrategy { PREFER_CACHE, ONLY_NETWORK, PREFER_NETWORK, + UPDATE_IN_BACKGROUND, } /** diff --git a/src/types/config.d.ts b/src/types/config.d.ts index f89ba6612..ceb4bb32c 100644 --- a/src/types/config.d.ts +++ b/src/types/config.d.ts @@ -71,4 +71,6 @@ export interface EnvironmentConfig { removeaccountonlogout?: boolean; // True to remove the account when the user clicks logout. Doesn't affect switch account. uselegacycompletion?: boolean; // Whether to use legacy completion by default in all course formats. toastDurations: Record; + disableCallWSInBackground?: boolean; // If true, disable calling WS in background. + callWSInBackgroundExpirationTime?: number; // Ms to consider an entry expired when calling WS in background. Default: 1 week. } From 3e462979f79f9c40b0090d1546bcae54a11eb33d Mon Sep 17 00:00:00 2001 From: Dani Palou Date: Thu, 23 Jun 2022 10:42:20 +0200 Subject: [PATCH 04/14] MOBILE-3817 dashboard: Create observable methods for getDashboardBlocks --- src/core/classes/site.ts | 11 +- .../features/courses/services/dashboard.ts | 146 ++++++++++++------ src/core/singletons/subscriptions.ts | 40 +++-- .../singletons/tests/subscriptions.test.ts | 49 ++++-- src/core/utils/observables.ts | 74 +++++++++ 5 files changed, 240 insertions(+), 80 deletions(-) create mode 100644 src/core/utils/observables.ts diff --git a/src/core/classes/site.ts b/src/core/classes/site.ts index 0fbc8c42d..894d27868 100644 --- a/src/core/classes/site.ts +++ b/src/core/classes/site.ts @@ -59,6 +59,7 @@ import { } from '@services/database/sites'; import { Observable, Subject } from 'rxjs'; import { finalize, map } from 'rxjs/operators'; +import { firstValueFrom } from '../utils/observables'; /** * QR Code type enumeration. @@ -494,7 +495,7 @@ export class CoreSite { */ // eslint-disable-next-line @typescript-eslint/no-explicit-any read(method: string, data: any, preSets?: CoreSiteWSPreSets): Promise { - return this.readObservable(method, data, preSets).toPromise(); + return firstValueFrom(this.readObservable(method, data, preSets)); } /** @@ -525,7 +526,7 @@ export class CoreSite { */ // eslint-disable-next-line @typescript-eslint/no-explicit-any write(method: string, data: any, preSets?: CoreSiteWSPreSets): Promise { - return this.writeObservable(method, data, preSets).toPromise(); + return firstValueFrom(this.writeObservable(method, data, preSets)); } /** @@ -556,7 +557,7 @@ export class CoreSite { */ // eslint-disable-next-line @typescript-eslint/no-explicit-any async request(method: string, data: any, preSets: CoreSiteWSPreSets): Promise { - return this.requestObservable(method, data, preSets).toPromise(); + return firstValueFrom(this.requestObservable(method, data, preSets)); } /** @@ -1640,7 +1641,7 @@ export class CoreSite { // Check for an ongoing identical request if we're not ignoring cache. if (cachePreSets.getFromCache && this.ongoingRequests[cacheId] !== undefined) { - return await this.ongoingRequests[cacheId].toPromise(); + return await firstValueFrom(this.ongoingRequests[cacheId]); } const subject = new Subject(); @@ -1698,7 +1699,7 @@ export class CoreSite { subject.error(error); }); - return observable.toPromise(); + return firstValueFrom(observable); } /** diff --git a/src/core/features/courses/services/dashboard.ts b/src/core/features/courses/services/dashboard.ts index 6e687c0e1..982c1a5cd 100644 --- a/src/core/features/courses/services/dashboard.ts +++ b/src/core/features/courses/services/dashboard.ts @@ -13,12 +13,15 @@ // limitations under the License. import { Injectable } from '@angular/core'; -import { CoreSites } from '@services/sites'; +import { CoreSites, CoreSitesCommonWSOptions } from '@services/sites'; import { CoreSite, CoreSiteWSPreSets } from '@classes/site'; import { CoreCourseBlock } from '@features/course/services/course'; import { CoreStatusWithWarningsWSResponse } from '@services/ws'; import { makeSingleton } from '@singletons'; import { CoreError } from '@classes/errors/error'; +import { Observable } from 'rxjs'; +import { map } from 'rxjs/operators'; +import { asyncObservable, firstValueFrom } from '@/core/utils/observables'; const ROOT_CACHE_KEY = 'CoreCoursesDashboard:'; @@ -51,40 +54,66 @@ export class CoreCoursesDashboardProvider { * @return Promise resolved with the list of blocks. * @since 3.6 */ - async getDashboardBlocksFromWS( + getDashboardBlocksFromWS( myPage = CoreCoursesDashboardProvider.MY_PAGE_DEFAULT, userId?: number, siteId?: string, ): Promise { - const site = await CoreSites.getSite(siteId); + return firstValueFrom(this.getDashboardBlocksFromWSObservable({ + myPage, + userId, + siteId, + })); + } - const params: CoreBlockGetDashboardBlocksWSParams = { - returncontents: true, - }; - if (CoreSites.getRequiredCurrentSite().isVersionGreaterEqualThan('4.0')) { - params.mypage = myPage; - } else if (myPage != CoreCoursesDashboardProvider.MY_PAGE_DEFAULT) { - throw new CoreError('mypage param is no accessible on core_block_get_dashboard_blocks'); - } + /** + * Get dashboard blocks from WS. + * + * @param options Options. + * @return Observable that returns the list of blocks. + * @since 3.6 + */ + getDashboardBlocksFromWSObservable(options: GetDashboardBlocksOptions = {}): Observable { + return asyncObservable(async () => { + const site = await CoreSites.getSite(options.siteId); - const preSets: CoreSiteWSPreSets = { - cacheKey: this.getDashboardBlocksCacheKey(myPage, userId), - updateFrequency: CoreSite.FREQUENCY_RARELY, - }; - if (userId) { - params.userid = userId; - } - const result = await site.read('core_block_get_dashboard_blocks', params, preSets); + const myPage = options.myPage ?? CoreCoursesDashboardProvider.MY_PAGE_DEFAULT; + const params: CoreBlockGetDashboardBlocksWSParams = { + returncontents: true, + }; + if (CoreSites.getRequiredCurrentSite().isVersionGreaterEqualThan('4.0')) { + params.mypage = myPage; + } else if (myPage != CoreCoursesDashboardProvider.MY_PAGE_DEFAULT) { + throw new CoreError('mypage param is no accessible on core_block_get_dashboard_blocks'); + } - if (site.isVersionGreaterEqualThan('4.0')) { - // Temporary hack to have course overview on 3.9.5 but not on 4.0 onwards. - // To be removed in a near future. - // Remove myoverview when is forced. See MDL-72092. - result.blocks = result.blocks.filter((block) => - block.instanceid != 0 || block.name != 'myoverview' || block.region != 'forced'); - } + const preSets: CoreSiteWSPreSets = { + cacheKey: this.getDashboardBlocksCacheKey(myPage, options.userId), + updateFrequency: CoreSite.FREQUENCY_RARELY, + ...CoreSites.getReadingStrategyPreSets(options.readingStrategy), + }; + if (options.userId) { + params.userid = options.userId; + } - return result.blocks || []; + const observable = site.readObservable( + 'core_block_get_dashboard_blocks', + params, + preSets, + ); + + return observable.pipe(map(result => { + if (site.isVersionGreaterEqualThan('4.0')) { + // Temporary hack to have course overview on 3.9.5 but not on 4.0 onwards. + // To be removed in a near future. + // Remove myoverview when is forced. See MDL-72092. + result.blocks = result.blocks.filter((block) => + block.instanceid != 0 || block.name != 'myoverview' || block.region != 'forced'); + } + + return result.blocks || []; + })); + }); } /** @@ -95,39 +124,52 @@ export class CoreCoursesDashboardProvider { * @param myPage What my page to return blocks of. Default MY_PAGE_DEFAULT. * @return Promise resolved with the list of blocks. */ - async getDashboardBlocks( + getDashboardBlocks( userId?: number, siteId?: string, myPage = CoreCoursesDashboardProvider.MY_PAGE_DEFAULT, ): Promise { - const blocks = await this.getDashboardBlocksFromWS(myPage, userId, siteId); + return firstValueFrom(this.getDashboardBlocksObservable({ + myPage, + userId, + siteId, + })); + } - let mainBlocks: CoreCourseBlock[] = []; - let sideBlocks: CoreCourseBlock[] = []; - - blocks.forEach((block) => { - if (block.region == 'content' || block.region == 'main') { - mainBlocks.push(block); - } else { - sideBlocks.push(block); - } - }); - - if (mainBlocks.length == 0) { - mainBlocks = []; - sideBlocks = []; + /** + * Get dashboard blocks. + * + * @param options Options. + * @return observable that returns the list of blocks. + */ + getDashboardBlocksObservable(options: GetDashboardBlocksOptions = {}): Observable { + return this.getDashboardBlocksFromWSObservable(options).pipe(map(blocks => { + let mainBlocks: CoreCourseBlock[] = []; + let sideBlocks: CoreCourseBlock[] = []; blocks.forEach((block) => { - if (block.region.match('side')) { - sideBlocks.push(block); - } else { + if (block.region == 'content' || block.region == 'main') { mainBlocks.push(block); + } else { + sideBlocks.push(block); } }); - } - return { mainBlocks, sideBlocks }; + if (mainBlocks.length == 0) { + mainBlocks = []; + sideBlocks = []; + blocks.forEach((block) => { + if (block.region.match('side')) { + sideBlocks.push(block); + } else { + mainBlocks.push(block); + } + }); + } + + return { mainBlocks, sideBlocks }; + })); } /** @@ -194,6 +236,14 @@ export type CoreCoursesDashboardBlocks = { sideBlocks: CoreCourseBlock[]; }; +/** + * Options for some get dashboard blocks calls. + */ +export type GetDashboardBlocksOptions = CoreSitesCommonWSOptions & { + userId?: number; // User ID. If not defined, current user. + myPage?: string; // Page to get. If not defined, CoreCoursesDashboardProvider.MY_PAGE_DEFAULT. +}; + /** * Params of core_block_get_dashboard_blocks WS. */ diff --git a/src/core/singletons/subscriptions.ts b/src/core/singletons/subscriptions.ts index 88e9dc1dd..f5c23b795 100644 --- a/src/core/singletons/subscriptions.ts +++ b/src/core/singletons/subscriptions.ts @@ -13,6 +13,7 @@ // limitations under the License. import { EventEmitter } from '@angular/core'; +import { CoreUtils } from '@services/utils/utils'; import { Observable, Subscription } from 'rxjs'; /** @@ -31,37 +32,46 @@ export class CoreSubscriptions { * @param subscribable Subscribable to listen to. * @param onSuccess Callback to run when the subscription is updated. * @param onError Callback to run when the an error happens. + * @param onComplete Callback to run when the observable completes. * @return A function to unsubscribe. */ static once( subscribable: Subscribable, onSuccess: (value: T) => unknown, onError?: (error: unknown) => unknown, + onComplete?: () => void, ): () => void { - let unsubscribe = false; + let callbackCalled = false; let subscription: Subscription | null = null; + const runCallback = (callback) => { + if (!callbackCalled) { + callbackCalled = true; + callback(); + } + }; + const unsubscribe = async () => { + // Subscription variable might not be set because we can receive a value immediately. Wait for next tick. + await CoreUtils.nextTick(); + + subscription?.unsubscribe(); + }; + subscription = subscribable.subscribe( value => { - // Subscription variable might not be set because we can receive a value immediately. - unsubscribe = true; - subscription?.unsubscribe(); - - onSuccess(value); + unsubscribe(); + runCallback(() => onSuccess(value)); }, error => { - // Subscription variable might not be set because we can receive a value immediately. - unsubscribe = true; - subscription?.unsubscribe(); - - onError && onError(error); + unsubscribe(); + runCallback(() => onError?.(error)); + }, + () => { + unsubscribe(); + runCallback(() => onComplete?.()); }, ); - if (unsubscribe) { - subscription.unsubscribe(); - } - return () => subscription?.unsubscribe(); } diff --git a/src/core/singletons/tests/subscriptions.test.ts b/src/core/singletons/tests/subscriptions.test.ts index 5ea863a8a..0ab324d89 100644 --- a/src/core/singletons/tests/subscriptions.test.ts +++ b/src/core/singletons/tests/subscriptions.test.ts @@ -17,12 +17,20 @@ import { BehaviorSubject, Subject } from 'rxjs'; describe('CoreSubscriptions singleton', () => { - it('calls callbacks only once', async () => { - // Test call success function. - let subject = new Subject(); - let success = jest.fn(); - let error = jest.fn(); - CoreSubscriptions.once(subject, success, error); + let subject: Subject; + let success: jest.Mock; + let error: jest.Mock; + let complete: jest.Mock; + + beforeEach(() => { + subject = new Subject(); + success = jest.fn(); + error = jest.fn(); + complete = jest.fn(); + }); + + it('calls success callback only once', async () => { + CoreSubscriptions.once(subject, success, error, complete); subject.next('foo'); expect(success).toHaveBeenCalledTimes(1); @@ -32,11 +40,11 @@ describe('CoreSubscriptions singleton', () => { subject.error('foo'); expect(success).toHaveBeenCalledTimes(1); expect(error).not.toHaveBeenCalled(); + expect(complete).not.toHaveBeenCalled(); + }); - // Test call error function. - subject = new Subject(); // Create a new Subject because the previous one already has an error. - success = jest.fn(); - CoreSubscriptions.once(subject, success, error); + it('calls error callback only once', async () => { + CoreSubscriptions.once(subject, success, error, complete); subject.error('foo'); expect(error).toHaveBeenCalledWith('foo'); @@ -45,11 +53,27 @@ describe('CoreSubscriptions singleton', () => { subject.error('bar'); expect(error).toHaveBeenCalledTimes(1); expect(success).not.toHaveBeenCalled(); + expect(complete).not.toHaveBeenCalled(); + }); + it('calls complete callback only once', async () => { + CoreSubscriptions.once(subject, success, error, complete); + + subject.complete(); + expect(complete).toHaveBeenCalled(); + + subject.next('foo'); + subject.error('bar'); + subject.complete(); + expect(complete).toHaveBeenCalledTimes(1); + expect(success).not.toHaveBeenCalled(); + expect(error).not.toHaveBeenCalled(); + }); + + it('calls success callback only once with behaviour subject', async () => { // Test with behaviour subject (success callback called immediately). const beaviourSubject = new BehaviorSubject('foo'); - error = jest.fn(); - CoreSubscriptions.once(beaviourSubject, success, error); + CoreSubscriptions.once(beaviourSubject, success, error, complete); expect(success).toHaveBeenCalledWith('foo'); @@ -57,6 +81,7 @@ describe('CoreSubscriptions singleton', () => { beaviourSubject.error('foo'); expect(success).toHaveBeenCalledTimes(1); expect(error).not.toHaveBeenCalled(); + expect(complete).not.toHaveBeenCalled(); }); it('allows unsubscribing from outside the once function', async () => { diff --git a/src/core/utils/observables.ts b/src/core/utils/observables.ts new file mode 100644 index 000000000..718fc884c --- /dev/null +++ b/src/core/utils/observables.ts @@ -0,0 +1,74 @@ +// (C) Copyright 2015 Moodle Pty Ltd. +// +// Licensed under the Apache License, Version 2.0 (the "License"); +// you may not use this file except in compliance with the License. +// You may obtain a copy of the License at +// +// http://www.apache.org/licenses/LICENSE-2.0 +// +// Unless required by applicable law or agreed to in writing, software +// distributed under the License is distributed on an "AS IS" BASIS, +// WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. +// See the License for the specific language governing permissions and +// limitations under the License. + +import { CoreError } from '@classes/errors/error'; +import { CoreSubscriptions } from '@singletons/subscriptions'; +import { BehaviorSubject, Observable, of } from 'rxjs'; +import { catchError } from 'rxjs/operators'; + +/** + * Convert to an Observable a Promise that resolves to an Observable. + * + * @param createObservable A function returning a promise that resolves to an Observable. + * @returns Observable. + */ +export function asyncObservable(createObservable: () => Promise>): Observable { + const promise = createObservable(); + + return new Observable(subscriber => { + promise + .then(observable => observable.subscribe( + value => subscriber.next(value), + error => subscriber.error(error), + () => subscriber.complete(), + )) + .catch(error => subscriber.error(error)); + }); +} + +/** + * Create a Promise that resolved with the first value returned from an observable. + * This function can be removed when the app starts using rxjs v7. + * + * @param observable Observable. + * @returns Promise resolved with the first value returned. + */ +export function firstValueFrom(observable: Observable): Promise { + return new Promise((resolve, reject) => { + CoreSubscriptions.once(observable, resolve, reject, () => { + // Subscription is completed, check if we can get its value. + if (observable instanceof BehaviorSubject) { + resolve(observable.getValue()); + } + + reject(new CoreError('Couldn\'t get first value from observable because it\'s already completed')); + }); + }); +} + +/** + * Ignore errors from an observable, returning a certain value instead. + * + * @param observable Observable to ignore errors. + * @param fallback Value to return if the observer errors. + * @return Observable with ignored errors, returning the fallback result if provided. + */ +export function ignoreErrors(observable: Observable): Observable; +export function ignoreErrors(observable: Observable, fallback: Fallback): Observable; +export function ignoreErrors( + observable: Observable, + fallback?: Fallback, +): Observable { + return observable.pipe(catchError(() => of(fallback))); +} From 73b108e5c52fcb3e93b247245ac022ee2bb6a1f7 Mon Sep 17 00:00:00 2001 From: Dani Palou Date: Thu, 30 Jun 2022 16:39:09 +0200 Subject: [PATCH 05/14] MOBILE-3817 core: Implement zipIncludingComplete and add tests --- .eslintrc.js | 1 + src/core/utils/observables.ts | 101 +++++++++++++- src/core/utils/tests/observables.test.ts | 160 +++++++++++++++++++++++ 3 files changed, 256 insertions(+), 6 deletions(-) create mode 100644 src/core/utils/tests/observables.test.ts diff --git a/.eslintrc.js b/.eslintrc.js index 6319501e3..865090c04 100644 --- a/.eslintrc.js +++ b/.eslintrc.js @@ -276,6 +276,7 @@ testsConfig['rules']['padded-blocks'] = [ }, ]; testsConfig['rules']['jest/expect-expect'] = 'off'; +testsConfig['rules']['jest/no-done-callback'] = 'off'; testsConfig['plugins'].push('jest'); testsConfig['extends'].push('plugin:jest/recommended'); diff --git a/src/core/utils/observables.ts b/src/core/utils/observables.ts index 718fc884c..9f30b1755 100644 --- a/src/core/utils/observables.ts +++ b/src/core/utils/observables.ts @@ -14,7 +14,7 @@ import { CoreError } from '@classes/errors/error'; import { CoreSubscriptions } from '@singletons/subscriptions'; -import { BehaviorSubject, Observable, of } from 'rxjs'; +import { BehaviorSubject, Observable, of, Subscription } from 'rxjs'; import { catchError } from 'rxjs/operators'; /** @@ -28,11 +28,7 @@ export function asyncObservable(createObservable: () => Promise return new Observable(subscriber => { promise - .then(observable => observable.subscribe( - value => subscriber.next(value), - error => subscriber.error(error), - () => subscriber.complete(), - )) + .then(observable => observable.subscribe(subscriber)) .catch(error => subscriber.error(error)); }); } @@ -72,3 +68,96 @@ export function ignoreErrors( ): Observable { return observable.pipe(catchError(() => of(fallback))); } + +/** + * Get return types of a list of observables. + */ +type GetObservablesReturnTypes = { [key in keyof T]: T[key] extends Observable ? R : never }; + +/** + * Data for an observable when zipping. + */ +type ZipObservableData = { // eslint-disable-line @typescript-eslint/no-explicit-any + values: T[]; + completed: boolean; + subscription?: Subscription; +}; + +/** + * Similar to rxjs zip operator, but once an observable completes we'll continue to emit the last value as long + * as the other observables continue to emit values. + * + * @param observables Observables to zip. + * @return Observable that emits the zipped values. + */ +export function zipIncudingComplete[]>( // eslint-disable-line @typescript-eslint/no-explicit-any + ...observables: T +): Observable> { + return new Observable(subscriber => { + const observablesData: ZipObservableData[] = []; + let nextIndex = 0; + let numCompleted = 0; + let hasErrored = false; + + // Treat an emitted event. + const treatEmitted = (completed = false) => { + if (hasErrored) { + return; + } + + if (numCompleted >= observables.length) { + subscriber.complete(); + + return; + } + + // Check if any observable still doesn't have data for the index. + const notReady = observablesData.some(data => !data.completed && data.values[nextIndex] === undefined); + if (notReady) { + return; + } + + // For each observable, get the value for the next index, or last value if not present (completed). + const valueToEmit = observablesData.map(observableData => + observableData.values[nextIndex] ?? observableData.values[observableData.values.length - 1]); + + nextIndex++; + subscriber.next(> valueToEmit); + + if (completed) { + // An observable was completed, there might be other values to emit. + treatEmitted(true); + } + }; + + observables.forEach((observable, obsIndex) => { + const observableData: ZipObservableData = { + values: [], + completed: false, + }; + + observableData.subscription = observable.subscribe({ + next: (value) => { + observableData.values.push(value); + treatEmitted(); + }, + error: (error) => { + hasErrored = true; + subscriber.error(error); + }, + complete: () => { + observableData.completed = true; + numCompleted++; + treatEmitted(true); + }, + }); + + observablesData[obsIndex] = observableData; + }); + + // When unsubscribing, unsubscribe from all observables. + return () => { + observablesData.forEach(observableData => observableData.subscription?.unsubscribe()); + }; + }); +} diff --git a/src/core/utils/tests/observables.test.ts b/src/core/utils/tests/observables.test.ts new file mode 100644 index 000000000..47a7a1e1c --- /dev/null +++ b/src/core/utils/tests/observables.test.ts @@ -0,0 +1,160 @@ +// (C) Copyright 2015 Moodle Pty Ltd. +// +// Licensed under the Apache License, Version 2.0 (the "License"); +// you may not use this file except in compliance with the License. +// You may obtain a copy of the License at +// +// http://www.apache.org/licenses/LICENSE-2.0 +// +// Unless required by applicable law or agreed to in writing, software +// distributed under the License is distributed on an "AS IS" BASIS, +// WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. +// See the License for the specific language governing permissions and +// limitations under the License. + +import { BehaviorSubject, Observable, Subject } from 'rxjs'; +import { TestScheduler } from 'rxjs/testing'; +import { asyncObservable, firstValueFrom, ignoreErrors, zipIncudingComplete } from '../observables'; + +describe('Observables utility functions', () => { + + it('asyncObservable emits values', (done) => { + const subject = new Subject(); + const promise = new Promise>((resolve) => { + resolve(subject); + }); + + asyncObservable(() => promise).subscribe({ + next: (value) => { + expect(value).toEqual('foo'); + done(); + }, + }); + + // Wait for the promise callback to be called before emitting the value. + setTimeout(() => subject.next('foo')); + }); + + it('asyncObservable emits errors', (done) => { + const subject = new Subject(); + const promise = new Promise>((resolve) => { + resolve(subject); + }); + + asyncObservable(() => promise).subscribe({ + error: (value) => { + expect(value).toEqual('foo'); + done(); + }, + }); + + // Wait for the promise callback to be called before emitting the value. + setTimeout(() => subject.error('foo')); + }); + + it('asyncObservable emits complete', (done) => { + const subject = new Subject(); + const promise = new Promise>((resolve) => { + resolve(subject); + }); + + asyncObservable(() => promise).subscribe({ + complete: () => done(), + }); + + // Wait for the promise callback to be called before emitting the value. + setTimeout(() => subject.complete()); + }); + + it('asyncObservable emits error if promise is rejected', (done) => { + const promise = new Promise>((resolve, reject) => { + reject('Custom error'); + }); + + asyncObservable(() => promise).subscribe({ + error: (error) => { + expect(error).toEqual('Custom error'); + done(); + }, + }); + }); + + it('returns first value emitted by an observable', async () => { + const subject = new Subject(); + setTimeout(() => subject.next('foo'), 10); + + await expect(firstValueFrom(subject)).resolves.toEqual('foo'); + + // Check that running it again doesn't get last value, it gets the new one. + setTimeout(() => subject.next('bar'), 10); + await expect(firstValueFrom(subject)).resolves.toEqual('bar'); + + // Check we cannot get first value if a subject is already completed. + subject.complete(); + await expect(firstValueFrom(subject)).rejects.toThrow(); + + // Check that we get last value when using BehaviourSubject. + const behaviorSubject = new BehaviorSubject('baz'); + await expect(firstValueFrom(behaviorSubject)).resolves.toEqual('baz'); + + // Check we get last value even if behaviour subject is completed. + behaviorSubject.complete(); + await expect(firstValueFrom(behaviorSubject)).resolves.toEqual('baz'); + + // Check that Promise is rejected if the observable emits an error. + const errorSubject = new Subject(); + setTimeout(() => errorSubject.error('foo error'), 10); + + await expect(firstValueFrom(errorSubject)).rejects.toMatch('foo error'); + }); + + it('ignores observable errors', (done) => { + const subject = new Subject(); + + ignoreErrors(subject, 'default value').subscribe({ + next: (value) => { + expect(value).toEqual('default value'); + done(); + }, + }); + + subject.error('foo'); + }); + + it('zips observables including complete events', () => { + const scheduler = new TestScheduler((actual, expected) => { + expect(actual).toEqual(expected); + }); + + scheduler.run(({ expectObservable, cold }) => { + expectObservable(zipIncudingComplete( + cold('-a-b---|', { + a: 'A1', + b: 'A2', + }), + cold('-a----b-c--|', { + a: 'B1', + b: 'B2', + c: 'B3', + }), + cold('-a-b-c--de-----|', { + a: 'C1', + b: 'C2', + c: 'C3', + d: 'C4', + e: 'C5', + }), + )).toBe( + '-a----b-c--(de)|', + { + a: ['A1','B1','C1'], + b: ['A2','B2','C2'], + c: ['A2','B3','C3'], + d: ['A2','B3','C4'], + e: ['A2','B3','C5'], + }, + ); + }); + }); + +}); From fbe46ee89530d388b488432f08944d6e25416b31 Mon Sep 17 00:00:00 2001 From: Dani Palou Date: Fri, 1 Jul 2022 14:48:55 +0200 Subject: [PATCH 06/14] MOBILE-3817 courses: Implement getUserCoursesWithOptionsObservable --- .../services/coursecompletion.ts | 69 ++- src/core/classes/site.ts | 68 ++- .../courses/services/courses-helper.ts | 203 ++++++--- src/core/features/courses/services/courses.ts | 405 +++++++++++------- 4 files changed, 518 insertions(+), 227 deletions(-) diff --git a/src/addons/coursecompletion/services/coursecompletion.ts b/src/addons/coursecompletion/services/coursecompletion.ts index 0fa9d22ec..162747136 100644 --- a/src/addons/coursecompletion/services/coursecompletion.ts +++ b/src/addons/coursecompletion/services/coursecompletion.ts @@ -14,13 +14,16 @@ import { Injectable } from '@angular/core'; import { CoreLogger } from '@singletons/logger'; -import { CoreSites } from '@services/sites'; +import { CoreSites, CoreSitesCommonWSOptions } from '@services/sites'; import { CoreUtils } from '@services/utils/utils'; import { CoreCourses } from '@features/courses/services/courses'; import { CoreSite, CoreSiteWSPreSets } from '@classes/site'; import { CoreStatusWithWarningsWSResponse, CoreWSExternalWarning } from '@services/ws'; import { makeSingleton } from '@singletons'; import { CoreError } from '@classes/errors/error'; +import { asyncObservable, firstValueFrom } from '@/core/utils/observables'; +import { Observable } from 'rxjs'; +import { map } from 'rxjs/operators'; const ROOT_CACHE_KEY = 'mmaCourseCompletion:'; @@ -93,33 +96,55 @@ export class AddonCourseCompletionProvider { * @param siteId Site ID. If not defined, use current site. * @return Promise to be resolved when the completion is retrieved. */ - async getCompletion( + getCompletion( courseId: number, userId?: number, preSets: CoreSiteWSPreSets = {}, siteId?: string, ): Promise { + return firstValueFrom(this.getCompletionObservable(courseId, { + userId, + preSets, + siteId, + })); + } - const site = await CoreSites.getSite(siteId); - userId = userId || site.getUserId(); - this.logger.debug('Get completion for course ' + courseId + ' and user ' + userId); + /** + * Get course completion status for a certain course and user. + * + * @param courseId Course ID. + * @param options Options. + * @return Observable returning the completion. + */ + getCompletionObservable( + courseId: number, + options: AddonCourseCompletionGetCompletionOptions = {}, + ): Observable { + return asyncObservable(async () => { + const site = await CoreSites.getSite(options.siteId); - const data: AddonCourseCompletionGetCourseCompletionStatusWSParams = { - courseid: courseId, - userid: userId, - }; + const userId = options.userId || site.getUserId(); + this.logger.debug('Get completion for course ' + courseId + ' and user ' + userId); - preSets.cacheKey = this.getCompletionCacheKey(courseId, userId); - preSets.updateFrequency = preSets.updateFrequency || CoreSite.FREQUENCY_SOMETIMES; - preSets.cacheErrors = ['notenroled']; + const data: AddonCourseCompletionGetCourseCompletionStatusWSParams = { + courseid: courseId, + userid: userId, + }; - const result: AddonCourseCompletionGetCourseCompletionStatusWSResponse = - await site.read('core_completion_get_course_completion_status', data, preSets); - if (result.completionstatus) { - return result.completionstatus; - } + const preSets = { + ...(options.preSets ?? {}), + cacheKey: this.getCompletionCacheKey(courseId, userId), + updateFrequency: CoreSite.FREQUENCY_SOMETIMES, + cacheErrors: ['notenroled'], + ...CoreSites.getReadingStrategyPreSets(options.readingStrategy), + }; - throw new CoreError('Cannot fetch course completion status'); + return site.readObservable( + 'core_completion_get_course_completion_status', + data, + preSets, + ).pipe(map(result => result.completionstatus)); + }); } /** @@ -312,3 +337,11 @@ export type AddonCourseCompletionGetCourseCompletionStatusWSResponse = { export type AddonCourseCompletionMarkCourseSelfCompletedWSParams = { courseid: number; // Course ID. }; + +/** + * Options for getCompletionObservable. + */ +export type AddonCourseCompletionGetCompletionOptions = CoreSitesCommonWSOptions & { + userId?: number; // Id of the user, default to current user. + preSets?: CoreSiteWSPreSets; // Presets to use when calling the WebService. +}; diff --git a/src/core/classes/site.ts b/src/core/classes/site.ts index 894d27868..a18a554f9 100644 --- a/src/core/classes/site.ts +++ b/src/core/classes/site.ts @@ -57,8 +57,8 @@ import { WSGroups, WS_CACHE_TABLES_PREFIX, } from '@services/database/sites'; -import { Observable, Subject } from 'rxjs'; -import { finalize, map } from 'rxjs/operators'; +import { Observable, ObservableInput, ObservedValueOf, OperatorFunction, Subject } from 'rxjs'; +import { finalize, map, mergeMap } from 'rxjs/operators'; import { firstValueFrom } from '../utils/observables'; /** @@ -2379,6 +2379,70 @@ export class CoreSite { } +/** + * Operator to chain requests when using observables. + * + * @param readingStrategy Reading strategy used for the current request. + * @param callback Callback called with the result of current request and the reading strategy to use in next requests. + * @return Operator. + */ +// eslint-disable-next-line @typescript-eslint/no-explicit-any +export function chainRequests>( + readingStrategy: CoreSitesReadingStrategy | undefined, + callback: (data: T, readingStrategy?: CoreSitesReadingStrategy) => O, +): OperatorFunction> { + return (source: Observable) => new Observable<{ data: T; readingStrategy?: CoreSitesReadingStrategy }>(subscriber => { + let firstValue = true; + let isCompleted = false; + + return source.subscribe({ + next: async (value) => { + if (readingStrategy !== CoreSitesReadingStrategy.UPDATE_IN_BACKGROUND) { + // Just use same strategy. + subscriber.next({ data: value, readingStrategy }); + + return; + } + + if (!firstValue) { + // Second (last) value. Chained requests should have used cached data already, just return 1 value now. + subscriber.next({ + data: value, + }); + + return; + } + + firstValue = false; + + // Wait to see if the observable is completed (no more values). + await CoreUtils.nextTick(); + + if (isCompleted) { + // Current request only returns cached data. Let chained requests update in background. + subscriber.next({ data: value, readingStrategy }); + } else { + // Current request will update in background. Prefer cached data in the chained requests. + subscriber.next({ + data: value, + readingStrategy: CoreSitesReadingStrategy.PREFER_CACHE, + }); + } + }, + error: (error) => subscriber.error(error), + complete: async () => { + isCompleted = true; + + await CoreUtils.nextTick(); + + subscriber.complete(); + }, + }); + }).pipe( + mergeMap(({ data, readingStrategy }) => callback(data, readingStrategy)), + ); +} + /** * PreSets accepted by the WS call. */ diff --git a/src/core/features/courses/services/courses-helper.ts b/src/core/features/courses/services/courses-helper.ts index 7b19a6945..546a38f3d 100644 --- a/src/core/features/courses/services/courses-helper.ts +++ b/src/core/features/courses/services/courses-helper.ts @@ -26,6 +26,10 @@ import { makeSingleton, Translate } from '@singletons'; import { CoreWSExternalFile } from '@services/ws'; import { AddonCourseCompletion } from '@addons/coursecompletion/services/coursecompletion'; import moment from 'moment-timezone'; +import { Observable, of } from 'rxjs'; +import { firstValueFrom, zipIncudingComplete } from '@/core/utils/observables'; +import { catchError, map } from 'rxjs/operators'; +import { chainRequests } from '@classes/site'; /** * Helper to gather some common courses functions. @@ -111,29 +115,47 @@ export class CoreCoursesHelperProvider { * @param loadCategoryNames Whether load category names or not. * @return Promise resolved when done. */ - async loadCoursesExtraInfo(courses: CoreEnrolledCourseDataWithExtraInfo[], loadCategoryNames: boolean = false): Promise { - if (!courses.length ) { - // No courses or cannot get the data, stop. - return; + loadCoursesExtraInfo( + courses: CoreEnrolledCourseDataWithExtraInfo[], + loadCategoryNames: boolean = false, + ): Promise { + return firstValueFrom(this.loadCoursesExtraInfoObservable(courses, loadCategoryNames)); + } + + /** + * Given a list of courses returned by core_enrol_get_users_courses, load some extra data using the WebService + * core_course_get_courses_by_field if available. + * + * @param courses List of courses. + * @param loadCategoryNames Whether load category names or not. + * @return Promise resolved when done. + */ + loadCoursesExtraInfoObservable( + courses: CoreEnrolledCourseDataWithExtraInfo[], + loadCategoryNames: boolean = false, + options: CoreSitesCommonWSOptions = {}, + ): Observable { + if (!courses.length) { + return of([]); } - let coursesInfo = {}; - let courseInfoAvailable = false; - - if (loadCategoryNames || (courses[0].overviewfiles === undefined && courses[0].displayname === undefined)) { - const courseIds = courses.map((course) => course.id).join(','); - - courseInfoAvailable = true; - - // Get the extra data for the courses. - const coursesInfosArray = await CoreCourses.getCoursesByField('ids', courseIds); - - coursesInfo = CoreUtils.arrayToObject(coursesInfosArray, 'id'); + if (!loadCategoryNames && (courses[0].overviewfiles !== undefined || courses[0].displayname !== undefined)) { + // No need to load more data. + return of(courses); } - courses.forEach((course) => { - this.loadCourseExtraInfo(course, courseInfoAvailable ? coursesInfo[course.id] : course, loadCategoryNames); - }); + const courseIds = courses.map((course) => course.id).join(','); + + // Get the extra data for the courses. + return CoreCourses.getCoursesByFieldObservable('ids', courseIds, options).pipe(map(coursesInfosArray => { + const coursesInfo = CoreUtils.arrayToObject(coursesInfosArray, 'id'); + + courses.forEach((course) => { + this.loadCourseExtraInfo(course, coursesInfo[course.id], loadCategoryNames); + }); + + return courses; + })); } /** @@ -196,43 +218,76 @@ export class CoreCoursesHelperProvider { * @param slice Slice results to get the X first one. If slice > 0 it will be done after sorting. * @param filter Filter using some field. * @param loadCategoryNames Whether load category names or not. + * @param options Options. * @return Courses filled with options. */ - async getUserCoursesWithOptions( + getUserCoursesWithOptions( sort: string = 'fullname', slice: number = 0, filter?: string, loadCategoryNames: boolean = false, options: CoreSitesCommonWSOptions = {}, ): Promise { - - let courses: CoreEnrolledCourseDataWithOptions[] = await CoreCourses.getUserCourses( - false, - options.siteId, - options.readingStrategy, - ); - if (courses.length <= 0) { - return []; - } - - const promises: Promise[] = []; - const courseIds = courses.map((course) => course.id); - - // Load course options of the course. - promises.push(CoreCourses.getCoursesAdminAndNavOptions(courseIds, options.siteId).then((options) => { - courses.forEach((course) => { - course.navOptions = options.navOptions[course.id]; - course.admOptions = options.admOptions[course.id]; - }); - - return; + return firstValueFrom(this.getUserCoursesWithOptionsObservable({ + sort, + slice, + filter, + loadCategoryNames, + ...options, })); + } - promises.push(this.loadCoursesExtraInfo(courses, loadCategoryNames)); + /** + * Get user courses with admin and nav options. + * + * @param options Options. + * @return Courses filled with options. + */ + getUserCoursesWithOptionsObservable( + options: CoreCoursesGetWithOptionsOptions = {}, + ): Observable { + return CoreCourses.getUserCoursesObservable(options).pipe( + chainRequests(options.readingStrategy, (courses, newReadingStrategy) => { + if (courses.length <= 0) { + return of([]); + } - await Promise.all(promises); + const courseIds = courses.map((course) => course.id); // Use all courses to get options, to use cache. + const newOptions = { + ...options, + readingStrategy: newReadingStrategy, + }; + courses = this.filterAndSortCoursesWithOptions(courses, options); - switch (filter) { + return zipIncudingComplete( + this.loadCoursesExtraInfoObservable(courses, options.loadCategoryNames, newOptions), + CoreCourses.getCoursesAdminAndNavOptionsObservable(courseIds, newOptions).pipe(map(courseOptions => { + courses.forEach((course: CoreEnrolledCourseDataWithOptions) => { + course.navOptions = courseOptions.navOptions[course.id]; + course.admOptions = courseOptions.admOptions[course.id]; + }); + })), + ...courses.map(course => this.loadCourseCompletedStatus(course, newOptions)), + ).pipe(map(() => courses)); + }), + ); + } + + /** + * Filter and sort some courses. + * + * @param courses Courses. + * @param options Options + * @return Courses filtered and sorted. + */ + protected filterAndSortCoursesWithOptions( + courses: CoreEnrolledCourseData[], + options: CoreCoursesGetWithOptionsOptions = {}, + ): CoreEnrolledCourseData[] { + const sort = options.sort ?? 'fullname'; + const slice = options.slice ?? -1; + + switch (options.filter) { case 'isfavourite': courses = courses.filter((course) => !!course.isfavourite); break; @@ -270,28 +325,42 @@ export class CoreCoursesHelperProvider { courses = slice > 0 ? courses.slice(0, slice) : courses; - return Promise.all(courses.map(async (course) => { - if (course.completed !== undefined) { - // The WebService already returns the completed status, no need to fetch it. + return courses; + } + + /** + * Given a course object, fetch and set its completed status if not present already. + * + * @param course Course. + * @return Observable. + */ + protected loadCourseCompletedStatus( + course: CoreEnrolledCourseDataWithExtraInfo, + options: CoreSitesCommonWSOptions = {}, + ): Observable { + if (course.completed !== undefined) { + // The WebService already returns the completed status, no need to fetch it. + return of(course); + } + + if (course.enablecompletion !== undefined && !course.enablecompletion) { + // Completion is disabled for this course, there is no need to fetch the completion status. + return of(course); + } + + return AddonCourseCompletion.getCompletionObservable(course.id, options).pipe( + map(completion => { + course.completed = completion.completed; + return course; - } - - if (course.enablecompletion !== undefined && !course.enablecompletion) { - // Completion is disabled for this course, there is no need to fetch the completion status. - return course; - } - - try { - const completion = await AddonCourseCompletion.getCompletion(course.id, undefined, undefined, options.siteId); - - course.completed = completion?.completed; - } catch { + }), + catchError(() => { // Ignore error, maybe course completion is disabled or user has no permission. course.completed = false; - } - return course; - })); + return of(course); + }), + ); } /** @@ -402,3 +471,13 @@ export type CoreCourseSearchedDataWithExtraInfoAndOptions = CoreCourseWithImageA export type CoreCourseAnyCourseDataWithExtraInfoAndOptions = CoreCourseWithImageAndColor & CoreCourseAnyCourseDataWithOptions & { categoryname?: string; // Category name, }; + +/** + * Options for getUserCoursesWithOptionsObservable. + */ +export type CoreCoursesGetWithOptionsOptions = CoreSitesCommonWSOptions & { + sort?: string; // Sort courses after get them. Defaults to 'fullname'. + slice?: number; // Slice results to get the X first one. If slice > 0 it will be done after sorting. + filter?: string; // Filter using some field. + loadCategoryNames?: boolean; // Whether load category names or not. +}; diff --git a/src/core/features/courses/services/courses.ts b/src/core/features/courses/services/courses.ts index 240e882cd..0f2d8c4a4 100644 --- a/src/core/features/courses/services/courses.ts +++ b/src/core/features/courses/services/courses.ts @@ -20,7 +20,9 @@ import { CoreStatusWithWarningsWSResponse, CoreWarningsWSResponse, CoreWSExterna import { CoreEvents } from '@singletons/events'; import { CoreWSError } from '@classes/errors/wserror'; import { CoreCourseAnyCourseDataWithExtraInfoAndOptions, CoreCourseWithImageAndColor } from './courses-helper'; -import { CoreUtils } from '@services/utils/utils'; +import { asyncObservable, firstValueFrom, ignoreErrors, zipIncudingComplete } from '@/core/utils/observables'; +import { of, Observable } from 'rxjs'; +import { map } from 'rxjs/operators'; const ROOT_CACHE_KEY = 'mmCourses:'; @@ -63,8 +65,7 @@ export class CoreCoursesProvider { static readonly STATE_HIDDEN = 'hidden'; static readonly STATE_FAVOURITE = 'favourite'; - protected userCoursesIds: { [id: number]: boolean } = {}; // Use an object to make it faster to search. - + protected userCoursesIds?: Set; protected downloadOptionsEnabled = false; /** @@ -484,60 +485,91 @@ export class CoreCoursesProvider { * @param siteId Site ID. If not defined, use current site. * @return Promise resolved with the courses. */ - async getCoursesByField( + getCoursesByField( field: string = '', value: string | number = '', siteId?: string, ): Promise { - siteId = siteId || CoreSites.getCurrentSiteId(); + return firstValueFrom(this.getCoursesByFieldObservable(field, value, { siteId })); + } - const originalValue = value; + /** + * Get courses. They can be filtered by field. + * + * @param field The field to search. Can be left empty for all courses or: + * id: course id. + * ids: comma separated course ids. + * shortname: course short name. + * idnumber: course id number. + * category: category id the course belongs to. + * @param value The value to match. + * @param options Other options. + * @return Observable that returns the courses. + */ + getCoursesByFieldObservable( + field: string = '', + value: string | number = '', + options: CoreSitesCommonWSOptions = {}, + ): Observable { + return asyncObservable(async () => { + const siteId = options.siteId || CoreSites.getCurrentSiteId(); + const originalValue = value; - const site = await CoreSites.getSite(siteId); + const site = await CoreSites.getSite(siteId); - const fieldParams = await this.fixCoursesByFieldParams(field, value, siteId); + // Fix params. Tries to use cached data, no need to use observer. + const fieldParams = await this.fixCoursesByFieldParams(field, value, siteId); - const hasChanged = fieldParams.field != field || fieldParams.value != value; - field = fieldParams.field; - value = fieldParams.value; - const data: CoreCourseGetCoursesByFieldWSParams = { - field: field, - value: field ? value : '', - }; - const preSets: CoreSiteWSPreSets = { - cacheKey: this.getCoursesByFieldCacheKey(field, value), - updateFrequency: CoreSite.FREQUENCY_RARELY, - }; + const hasChanged = fieldParams.field != field || fieldParams.value != value; + field = fieldParams.field; + value = fieldParams.value; + const data: CoreCourseGetCoursesByFieldWSParams = { + field: field, + value: field ? value : '', + }; + const preSets: CoreSiteWSPreSets = { + cacheKey: this.getCoursesByFieldCacheKey(field, value), + updateFrequency: CoreSite.FREQUENCY_RARELY, + ...CoreSites.getReadingStrategyPreSets(options.readingStrategy), + }; - const response = await site.read('core_course_get_courses_by_field', data, preSets); - if (!response.courses) { - throw Error('WS core_course_get_courses_by_field failed'); - } + const observable = site.readObservable( + 'core_course_get_courses_by_field', + data, + preSets, + ); - if (field == 'ids' && hasChanged) { - // The list of courses requestes was changed to optimize it. - // Return only the ones that were being requested. - const courseIds = String(originalValue).split(',').map((id) => parseInt(id, 10)); + return observable.pipe(map(response => { + if (!response.courses) { + throw Error('WS core_course_get_courses_by_field failed'); + } - // Only courses from the original selection. - response.courses = response.courses.filter((course) => courseIds.indexOf(course.id) >= 0); - } + if (field == 'ids' && hasChanged) { + // The list of courses requestes was changed to optimize it. + // Return only the ones that were being requested. + const courseIds = String(originalValue).split(',').map((id) => parseInt(id, 10)); - // Courses will be sorted using sortorder if available. - return response.courses.sort((a, b) => { - if (a.sortorder === undefined && b.sortorder === undefined) { - return b.id - a.id; - } + // Only courses from the original selection. + response.courses = response.courses.filter((course) => courseIds.indexOf(course.id) >= 0); + } - if (a.sortorder === undefined) { - return 1; - } + // Courses will be sorted using sortorder if available. + return response.courses.sort((a, b) => { + if (a.sortorder === undefined && b.sortorder === undefined) { + return b.id - a.id; + } - if (b.sortorder === undefined) { - return -1; - } + if (a.sortorder === undefined) { + return 1; + } - return a.sortorder - b.sortorder; + if (b.sortorder === undefined) { + return -1; + } + + return a.sortorder - b.sortorder; + }); + })); }); } @@ -614,25 +646,45 @@ export class CoreCoursesProvider { * @param siteId Site ID. If not defined, current site. * @return Promise resolved with the options for each course. */ - async getCoursesAdminAndNavOptions( + getCoursesAdminAndNavOptions( courseIds: number[], siteId?: string, ): Promise<{ navOptions: CoreCourseUserAdminOrNavOptionCourseIndexed; admOptions: CoreCourseUserAdminOrNavOptionCourseIndexed; }> { - siteId = siteId || CoreSites.getCurrentSiteId(); + return firstValueFrom(this.getCoursesAdminAndNavOptionsObservable(courseIds, { siteId })); + } - // Get the list of courseIds to use based on the param. - courseIds = await this.getCourseIdsForAdminAndNavOptions(courseIds, siteId); + /** + * Get the navigation and administration options for the given courses. + * + * @param courseIds IDs of courses to get. + * @param options Options. + * @return Observable that returns the options for each course. + */ + getCoursesAdminAndNavOptionsObservable( + courseIds: number[], + options: CoreSitesCommonWSOptions = {}, + ): Observable<{ + navOptions: CoreCourseUserAdminOrNavOptionCourseIndexed; + admOptions: CoreCourseUserAdminOrNavOptionCourseIndexed; + }> { - // Get user navigation and administration options. - const [navOptions, admOptions] = await Promise.all([ - CoreUtils.ignoreErrors(this.getUserNavigationOptions(courseIds, siteId), {}), - CoreUtils.ignoreErrors(this.getUserAdministrationOptions(courseIds, siteId), {}), - ]); + return asyncObservable(async () => { + const siteId = options.siteId || CoreSites.getCurrentSiteId(); - return { navOptions: navOptions, admOptions: admOptions }; + // Get the list of courseIds to use based on the param. Tries to use cached data, no need to use observer. + courseIds = await this.getCourseIdsForAdminAndNavOptions(courseIds, siteId); + + // Get user navigation and administration options. + return zipIncudingComplete( + ignoreErrors(this.getUserNavigationOptionsObservable(courseIds, options), {}), + ignoreErrors(this.getUserAdministrationOptionsObservable(courseIds, options), {}), + ).pipe( + map(([navOptions, admOptions]) => ({ navOptions, admOptions })), + ); + }); } /** @@ -695,26 +747,46 @@ export class CoreCoursesProvider { * @param siteId Site ID. If not defined, current site. * @return Promise resolved with administration options for each course. */ - async getUserAdministrationOptions(courseIds: number[], siteId?: string): Promise { + getUserAdministrationOptions(courseIds: number[], siteId?: string): Promise { + return firstValueFrom(this.getUserAdministrationOptionsObservable(courseIds, { siteId })); + } + + /** + * Get user administration options for a set of courses. + * + * @param courseIds IDs of courses to get. + * @param options Options. + * @return Observable that returns administration options for each course. + */ + getUserAdministrationOptionsObservable( + courseIds: number[], + options: CoreSitesCommonWSOptions = {}, + ): Observable { if (!courseIds || courseIds.length == 0) { - return {}; + return of({}); } - const site = await CoreSites.getSite(siteId); + return asyncObservable(async () => { + const site = await CoreSites.getSite(options.siteId); - const params: CoreCourseGetUserAdminOrNavOptionsWSParams = { - courseids: courseIds, - }; - const preSets: CoreSiteWSPreSets = { - cacheKey: this.getUserAdministrationOptionsCacheKey(courseIds), - updateFrequency: CoreSite.FREQUENCY_RARELY, - }; + const params: CoreCourseGetUserAdminOrNavOptionsWSParams = { + courseids: courseIds, + }; + const preSets: CoreSiteWSPreSets = { + cacheKey: this.getUserAdministrationOptionsCacheKey(courseIds), + updateFrequency: CoreSite.FREQUENCY_RARELY, + ...CoreSites.getReadingStrategyPreSets(options.readingStrategy), + }; - const response: CoreCourseGetUserAdminOrNavOptionsWSResponse = - await site.read('core_course_get_user_administration_options', params, preSets); + const observable = site.readObservable( + 'core_course_get_user_administration_options', + params, + preSets, + ); - // Format returned data. - return this.formatUserAdminOrNavOptions(response.courses); + // Format returned data. + return observable.pipe(map(response => this.formatUserAdminOrNavOptions(response.courses))); + }); } /** @@ -743,25 +815,45 @@ export class CoreCoursesProvider { * @return Promise resolved with navigation options for each course. */ async getUserNavigationOptions(courseIds: number[], siteId?: string): Promise { + return firstValueFrom(this.getUserNavigationOptionsObservable(courseIds, { siteId })); + } + + /** + * Get user navigation options for a set of courses. + * + * @param courseIds IDs of courses to get. + * @param options Options. + * @return Observable that returns navigation options for each course. + */ + getUserNavigationOptionsObservable( + courseIds: number[], + options: CoreSitesCommonWSOptions = {}, + ): Observable { if (!courseIds || courseIds.length == 0) { - return {}; + return of({}); } - const site = await CoreSites.getSite(siteId); + return asyncObservable(async () => { + const site = await CoreSites.getSite(options.siteId); - const params: CoreCourseGetUserAdminOrNavOptionsWSParams = { - courseids: courseIds, - }; - const preSets: CoreSiteWSPreSets = { - cacheKey: this.getUserNavigationOptionsCacheKey(courseIds), - updateFrequency: CoreSite.FREQUENCY_RARELY, - }; + const params: CoreCourseGetUserAdminOrNavOptionsWSParams = { + courseids: courseIds, + }; + const preSets: CoreSiteWSPreSets = { + cacheKey: this.getUserNavigationOptionsCacheKey(courseIds), + updateFrequency: CoreSite.FREQUENCY_RARELY, + ...CoreSites.getReadingStrategyPreSets(options.readingStrategy), + }; - const response: CoreCourseGetUserAdminOrNavOptionsWSResponse = - await site.read('core_course_get_user_navigation_options', params, preSets); + const observable = site.readObservable( + 'core_course_get_user_navigation_options', + params, + preSets, + ); - // Format returned data. - return this.formatUserAdminOrNavOptions(response.courses); + // Format returned data. + return observable.pipe(map(response => this.formatUserAdminOrNavOptions(response.courses))); + }); } /** @@ -818,89 +910,112 @@ export class CoreCoursesProvider { * * @param preferCache True if shouldn't call WS if data is cached, false otherwise. * @param siteId Site to get the courses from. If not defined, use current site. + * @param strategy Reading strategy. * @return Promise resolved with the courses. */ - async getUserCourses( + getUserCourses( preferCache: boolean = false, siteId?: string, strategy?: CoreSitesReadingStrategy, ): Promise { - const site = await CoreSites.getSite(siteId); + strategy = strategy ?? (preferCache ? CoreSitesReadingStrategy.PREFER_CACHE : undefined); - const userId = site.getUserId(); - const wsParams: CoreEnrolGetUsersCoursesWSParams = { - userid: userId, - }; - const strategyPreSets = strategy - ? CoreSites.getReadingStrategyPreSets(strategy) - : { omitExpires: !!preferCache }; + return this.getUserCoursesObservable({ + readingStrategy: strategy, + siteId, + }).toPromise(); + } - const preSets = { - cacheKey: this.getUserCoursesCacheKey(), - getCacheUsingCacheKey: true, - updateFrequency: CoreSite.FREQUENCY_RARELY, - ...strategyPreSets, - }; + /** + * Get user courses. + * + * @param options Options. + * @return Observable that returns the courses. + */ + getUserCoursesObservable(options: CoreSitesCommonWSOptions = {}): Observable { + return asyncObservable(async () => { + const site = await CoreSites.getSite(options.siteId); - if (site.isVersionGreaterEqualThan('3.7')) { - wsParams.returnusercount = false; - } + const userId = site.getUserId(); + const wsParams: CoreEnrolGetUsersCoursesWSParams = { + userid: userId, + }; - const courses = await site.read('core_enrol_get_users_courses', wsParams, preSets); + const preSets: CoreSiteWSPreSets = { + cacheKey: this.getUserCoursesCacheKey(), + getCacheUsingCacheKey: true, + updateFrequency: CoreSite.FREQUENCY_RARELY, + ...CoreSites.getReadingStrategyPreSets(options.readingStrategy), + }; - if (this.userCoursesIds) { - // Check if the list of courses has changed. - const added: number[] = []; - const removed: number[] = []; - const previousIds = Object.keys(this.userCoursesIds); - const currentIds = {}; // Use an object to make it faster to search. + if (site.isVersionGreaterEqualThan('3.7')) { + wsParams.returnusercount = false; + } - courses.forEach((course) => { - // Move category field to categoryid on a course. - course.categoryid = course.category; - delete course.category; + const observable = site.readObservable( + 'core_enrol_get_users_courses', + wsParams, + preSets, + ); - currentIds[course.id] = true; + return observable.pipe(map(courses => { + if (this.userCoursesIds) { + // Check if the list of courses has changed. + const added: number[] = []; + const removed: number[] = []; + const previousIds = this.userCoursesIds; + const currentIds = new Set(); - if (!this.userCoursesIds[course.id]) { - // Course added. - added.push(course.id); - } - }); + courses.forEach((course) => { + // Move category field to categoryid on a course. + course.categoryid = course.category; + delete course.category; - if (courses.length - added.length != previousIds.length) { - // A course was removed, check which one. - previousIds.forEach((id) => { - if (!currentIds[id]) { - // Course removed. - removed.push(Number(id)); + currentIds.add(course.id); + + if (!previousIds.has(course.id)) { + // Course added. + added.push(course.id); + } + }); + + if (courses.length - added.length !== previousIds.size) { + // A course was removed, check which one. + previousIds.forEach((id) => { + if (!currentIds.has(id)) { + // Course removed. + removed.push(Number(id)); + } + }); } - }); - } - if (added.length || removed.length) { - // At least 1 course was added or removed, trigger the event. - CoreEvents.trigger(CoreCoursesProvider.EVENT_MY_COURSES_CHANGED, { - added: added, - removed: removed, - }, site.getId()); - } + if (added.length || removed.length) { + // At least 1 course was added or removed, trigger the event. + CoreEvents.trigger(CoreCoursesProvider.EVENT_MY_COURSES_CHANGED, { + added: added, + removed: removed, + }, site.getId()); + } - this.userCoursesIds = currentIds; - } else { - this.userCoursesIds = {}; + this.userCoursesIds = currentIds; + } else { + const coursesIds = new Set(); - // Store the list of courses. - courses.forEach((course) => { - // Move category field to categoryid on a course. - course.categoryid = course.category; - delete course.category; + // Store the list of courses. + courses.forEach((course) => { + coursesIds.add(course.id); - this.userCoursesIds[course.id] = true; - }); - } + // Move category field to categoryid on a course. + course.categoryid = course.category; + delete course.category; + }); - return courses; + this.userCoursesIds = coursesIds; + } + + return courses; + })); + }); } /** @@ -1312,12 +1427,12 @@ export type CoreEnrolledCourseData = CoreEnrolledCourseBasicData & { completionhascriteria?: boolean; // If completion criteria is set. completionusertracked?: boolean; // If the user is completion tracked. progress?: number | null; // Progress percentage. - completed?: boolean; // Whether the course is completed. - marker?: number; // Course section marker. - lastaccess?: number; // Last access to the course (timestamp). + completed?: boolean; // @since 3.6. Whether the course is completed. + marker?: number; // @since 3.6. Course section marker. + lastaccess?: number; // @since 3.6. Last access to the course (timestamp). isfavourite?: boolean; // If the user marked this course a favourite. hidden?: boolean; // If the user hide the course from the dashboard. - overviewfiles?: CoreWSExternalFile[]; + overviewfiles?: CoreWSExternalFile[]; // @since 3.6. showactivitydates?: boolean; // @since 3.11. Whether the activity dates are shown or not. showcompletionconditions?: boolean; // @since 3.11. Whether the activity completion conditions are shown or not. timemodified?: number; // @since 4.0. Last time course settings were updated (timestamp). From c9a0b372a925e88fe63a8d132bec9b59322ed0ee Mon Sep 17 00:00:00 2001 From: Dani Palou Date: Tue, 5 Jul 2022 15:10:36 +0200 Subject: [PATCH 07/14] MOBILE-3817 myoverview: Save setting when using custom filter --- .../components/myoverview/myoverview.ts | 26 +++++++++++++++---- 1 file changed, 21 insertions(+), 5 deletions(-) diff --git a/src/addons/block/myoverview/components/myoverview/myoverview.ts b/src/addons/block/myoverview/components/myoverview/myoverview.ts index 9bdbffe47..53105110d 100644 --- a/src/addons/block/myoverview/components/myoverview/myoverview.ts +++ b/src/addons/block/myoverview/components/myoverview/myoverview.ts @@ -252,7 +252,7 @@ export class AddonBlockMyOverviewComponent extends CoreBlockBaseComponent implem this.loadSort(); this.loadLayouts(config?.layouts?.value.split(',')); - this.loadFilters(config); + await this.loadFilters(config); this.isDirty = false; } @@ -280,9 +280,9 @@ export class AddonBlockMyOverviewComponent extends CoreBlockBaseComponent implem * * @param config Block configuration. */ - protected loadFilters( + protected async loadFilters( config?: Record, - ): void { + ): Promise { if (!this.hasCourses) { return; } @@ -320,7 +320,7 @@ export class AddonBlockMyOverviewComponent extends CoreBlockBaseComponent implem this.saveFilters('all'); } - this.filterCourses(); + await this.filterCourses(); } /** @@ -369,6 +369,8 @@ export class AddonBlockMyOverviewComponent extends CoreBlockBaseComponent implem protected async refreshCourseList(data: CoreCoursesMyCoursesUpdatedEventData): Promise { if (data.action == CoreCoursesProvider.ACTION_ENROL) { // Always update if user enrolled in a course. + this.loaded = false; + return this.refreshContent(); } @@ -376,6 +378,8 @@ export class AddonBlockMyOverviewComponent extends CoreBlockBaseComponent implem if (data.action == CoreCoursesProvider.ACTION_STATE_CHANGED) { if (!course) { // Not found, use WS update. + this.loaded = false; + return this.refreshContent(); } @@ -394,6 +398,8 @@ export class AddonBlockMyOverviewComponent extends CoreBlockBaseComponent implem if (data.action == CoreCoursesProvider.ACTION_VIEW && data.courseId != CoreSites.getCurrentSiteHomeId()) { if (!course) { // Not found, use WS update. + this.loaded = false; + return this.refreshContent(); } @@ -463,7 +469,9 @@ export class AddonBlockMyOverviewComponent extends CoreBlockBaseComponent implem const customFilterValue = this.filters.customFilters[timeFilter.substring(7)]?.value; if (customFilterName !== undefined && customFilterValue !== undefined) { + const alreadyLoading = this.loaded === false; this.loaded = false; + try { const courses = await CoreCourses.getEnrolledCoursesByCustomField(customFilterName, customFilterValue); @@ -471,10 +479,18 @@ export class AddonBlockMyOverviewComponent extends CoreBlockBaseComponent implem const courseIds = courses.map((course) => course.id); this.filteredCourses = this.filteredCourses.filter((course) => courseIds.includes(course.id)); + this.saveFilters(timeFilter); } catch (error) { + if (alreadyLoading) { + throw error; // Pass the error to the caller so it's treated there. + } + CoreDomUtils.showErrorModalDefault(error, this.fetchContentDefaultError); } finally { - this.loaded = true; + if (!alreadyLoading) { + // Only set loaded to true if there was no other data being loaded. + this.loaded = true; + } } } } else { From 88297ed4006946f7a9085abcf4c9f8a7b4213df4 Mon Sep 17 00:00:00 2001 From: Dani Palou Date: Wed, 6 Jul 2022 08:02:41 +0200 Subject: [PATCH 08/14] MOBILE-3817 core: Implement more observable funcs for myoverview --- src/core/classes/site.ts | 26 +++++--- src/core/features/courses/services/courses.ts | 59 ++++++++++++------- 2 files changed, 58 insertions(+), 27 deletions(-) diff --git a/src/core/classes/site.ts b/src/core/classes/site.ts index a18a554f9..9e233beb9 100644 --- a/src/core/classes/site.ts +++ b/src/core/classes/site.ts @@ -1890,16 +1890,28 @@ export class CoreSite { getConfig(name?: undefined, ignoreCache?: boolean): Promise; getConfig(name: string, ignoreCache?: boolean): Promise; getConfig(name?: string, ignoreCache?: boolean): Promise { + return firstValueFrom( + this.getConfigObservable( name, ignoreCache ? CoreSitesReadingStrategy.ONLY_NETWORK : undefined), + ); + } + + /** + * Get the config of this site. + * It is recommended to use getStoredConfig instead since it's faster and doesn't use network. + * + * @param name Name of the setting to get. If not set or false, all settings will be returned. + * @param readingStrategy Reading strategy. + * @return Observable returning site config. + */ + getConfigObservable(name?: undefined, readingStrategy?: CoreSitesReadingStrategy): Observable; + getConfigObservable(name: string, readingStrategy?: CoreSitesReadingStrategy): Observable; + getConfigObservable(name?: string, readingStrategy?: CoreSitesReadingStrategy): Observable { const preSets: CoreSiteWSPreSets = { cacheKey: this.getConfigCacheKey(), + ...CoreSites.getReadingStrategyPreSets(readingStrategy), }; - if (ignoreCache) { - preSets.getFromCache = false; - preSets.emergencyCache = false; - } - - return this.read('tool_mobile_get_config', {}, preSets).then((config: CoreSiteConfigResponse) => { + return this.readObservable('tool_mobile_get_config', {}, preSets).pipe(map(config => { if (name) { // Return the requested setting. for (const x in config.settings) { @@ -1918,7 +1930,7 @@ export class CoreSite { return settings; } - }); + })); } /** diff --git a/src/core/features/courses/services/courses.ts b/src/core/features/courses/services/courses.ts index 0f2d8c4a4..fea06c2bc 100644 --- a/src/core/features/courses/services/courses.ts +++ b/src/core/features/courses/services/courses.ts @@ -585,7 +585,7 @@ export class CoreCoursesProvider { } /** - * Get courses matching the given custom field. Only works in online. + * Get courses matching the given custom field. By default it will try not to use cache. * * @param customFieldName Custom field name. * @param customFieldValue Custom field value. @@ -593,30 +593,49 @@ export class CoreCoursesProvider { * @return Promise resolved with the list of courses. * @since 3.8 */ - async getEnrolledCoursesByCustomField( + getEnrolledCoursesByCustomField( customFieldName: string, customFieldValue: string, siteId?: string, ): Promise { - const site = await CoreSites.getSite(siteId); - const params: CoreCourseGetEnrolledCoursesByTimelineClassificationWSParams = { - classification: 'customfield', - customfieldname: customFieldName, - customfieldvalue: customFieldValue, - }; - const preSets: CoreSiteWSPreSets = { - getFromCache: false, - }; - const courses = await site.read( - 'core_course_get_enrolled_courses_by_timeline_classification', - params, - preSets, - ); - if (courses.courses) { - return courses.courses; - } + return firstValueFrom(this.getEnrolledCoursesByCustomFieldObservable(customFieldName, customFieldValue, { + readingStrategy: CoreSitesReadingStrategy.PREFER_NETWORK, + siteId, + })); + } - throw Error('WS core_course_get_enrolled_courses_by_timeline_classification failed'); + /** + * Get courses matching the given custom field. + * + * @param customFieldName Custom field name. + * @param customFieldValue Custom field value. + * @param options Common options. + * @return Promise resolved with the list of courses. + * @since 3.8 + */ + getEnrolledCoursesByCustomFieldObservable( + customFieldName: string, + customFieldValue: string, + options: CoreSitesCommonWSOptions, + ): Observable { + return asyncObservable(async () => { + const site = await CoreSites.getSite(options. siteId); + + const params: CoreCourseGetEnrolledCoursesByTimelineClassificationWSParams = { + classification: 'customfield', + customfieldname: customFieldName, + customfieldvalue: customFieldValue, + }; + const preSets: CoreSiteWSPreSets = { + ...CoreSites.getReadingStrategyPreSets(options.readingStrategy ?? CoreSitesReadingStrategy.PREFER_NETWORK), + }; + + return site.readObservable( + 'core_course_get_enrolled_courses_by_timeline_classification', + params, + preSets, + ).pipe(map(response => response.courses)); + }); } /** From 63f3c440e34beacfe7e99fc0fdf5aa34dcf484db Mon Sep 17 00:00:00 2001 From: Dani Palou Date: Mon, 18 Jul 2022 12:22:38 +0200 Subject: [PATCH 09/14] MOBILE-3817 utils: Move new utils functions to existing rxjs file --- .../services/coursecompletion.ts | 2 +- src/core/classes/site.ts | 2 +- .../courses/services/courses-helper.ts | 4 +- src/core/features/courses/services/courses.ts | 4 +- .../features/courses/services/dashboard.ts | 2 +- src/core/utils/observables.ts | 163 ------------------ src/core/utils/rxjs.ts | 152 +++++++++++++++- src/core/utils/tests/observables.test.ts | 160 ----------------- src/core/utils/tests/rxjs.test.ts | 158 ++++++++++++++++- 9 files changed, 310 insertions(+), 337 deletions(-) delete mode 100644 src/core/utils/observables.ts delete mode 100644 src/core/utils/tests/observables.test.ts diff --git a/src/addons/coursecompletion/services/coursecompletion.ts b/src/addons/coursecompletion/services/coursecompletion.ts index 162747136..76feeecd4 100644 --- a/src/addons/coursecompletion/services/coursecompletion.ts +++ b/src/addons/coursecompletion/services/coursecompletion.ts @@ -21,7 +21,7 @@ import { CoreSite, CoreSiteWSPreSets } from '@classes/site'; import { CoreStatusWithWarningsWSResponse, CoreWSExternalWarning } from '@services/ws'; import { makeSingleton } from '@singletons'; import { CoreError } from '@classes/errors/error'; -import { asyncObservable, firstValueFrom } from '@/core/utils/observables'; +import { asyncObservable, firstValueFrom } from '@/core/utils/rxjs'; import { Observable } from 'rxjs'; import { map } from 'rxjs/operators'; diff --git a/src/core/classes/site.ts b/src/core/classes/site.ts index 9e233beb9..ec047eb8a 100644 --- a/src/core/classes/site.ts +++ b/src/core/classes/site.ts @@ -59,7 +59,7 @@ import { } from '@services/database/sites'; import { Observable, ObservableInput, ObservedValueOf, OperatorFunction, Subject } from 'rxjs'; import { finalize, map, mergeMap } from 'rxjs/operators'; -import { firstValueFrom } from '../utils/observables'; +import { firstValueFrom } from '../utils/rxjs'; /** * QR Code type enumeration. diff --git a/src/core/features/courses/services/courses-helper.ts b/src/core/features/courses/services/courses-helper.ts index 546a38f3d..a4231cec5 100644 --- a/src/core/features/courses/services/courses-helper.ts +++ b/src/core/features/courses/services/courses-helper.ts @@ -27,7 +27,7 @@ import { CoreWSExternalFile } from '@services/ws'; import { AddonCourseCompletion } from '@addons/coursecompletion/services/coursecompletion'; import moment from 'moment-timezone'; import { Observable, of } from 'rxjs'; -import { firstValueFrom, zipIncudingComplete } from '@/core/utils/observables'; +import { firstValueFrom, zipIncludingComplete } from '@/core/utils/rxjs'; import { catchError, map } from 'rxjs/operators'; import { chainRequests } from '@classes/site'; @@ -259,7 +259,7 @@ export class CoreCoursesHelperProvider { }; courses = this.filterAndSortCoursesWithOptions(courses, options); - return zipIncudingComplete( + return zipIncludingComplete( this.loadCoursesExtraInfoObservable(courses, options.loadCategoryNames, newOptions), CoreCourses.getCoursesAdminAndNavOptionsObservable(courseIds, newOptions).pipe(map(courseOptions => { courses.forEach((course: CoreEnrolledCourseDataWithOptions) => { diff --git a/src/core/features/courses/services/courses.ts b/src/core/features/courses/services/courses.ts index fea06c2bc..6c4ea9e43 100644 --- a/src/core/features/courses/services/courses.ts +++ b/src/core/features/courses/services/courses.ts @@ -20,7 +20,7 @@ import { CoreStatusWithWarningsWSResponse, CoreWarningsWSResponse, CoreWSExterna import { CoreEvents } from '@singletons/events'; import { CoreWSError } from '@classes/errors/wserror'; import { CoreCourseAnyCourseDataWithExtraInfoAndOptions, CoreCourseWithImageAndColor } from './courses-helper'; -import { asyncObservable, firstValueFrom, ignoreErrors, zipIncudingComplete } from '@/core/utils/observables'; +import { asyncObservable, firstValueFrom, ignoreErrors, zipIncludingComplete } from '@/core/utils/rxjs'; import { of, Observable } from 'rxjs'; import { map } from 'rxjs/operators'; @@ -697,7 +697,7 @@ export class CoreCoursesProvider { courseIds = await this.getCourseIdsForAdminAndNavOptions(courseIds, siteId); // Get user navigation and administration options. - return zipIncudingComplete( + return zipIncludingComplete( ignoreErrors(this.getUserNavigationOptionsObservable(courseIds, options), {}), ignoreErrors(this.getUserAdministrationOptionsObservable(courseIds, options), {}), ).pipe( diff --git a/src/core/features/courses/services/dashboard.ts b/src/core/features/courses/services/dashboard.ts index 982c1a5cd..eb812b835 100644 --- a/src/core/features/courses/services/dashboard.ts +++ b/src/core/features/courses/services/dashboard.ts @@ -21,7 +21,7 @@ import { makeSingleton } from '@singletons'; import { CoreError } from '@classes/errors/error'; import { Observable } from 'rxjs'; import { map } from 'rxjs/operators'; -import { asyncObservable, firstValueFrom } from '@/core/utils/observables'; +import { asyncObservable, firstValueFrom } from '@/core/utils/rxjs'; const ROOT_CACHE_KEY = 'CoreCoursesDashboard:'; diff --git a/src/core/utils/observables.ts b/src/core/utils/observables.ts deleted file mode 100644 index 9f30b1755..000000000 --- a/src/core/utils/observables.ts +++ /dev/null @@ -1,163 +0,0 @@ -// (C) Copyright 2015 Moodle Pty Ltd. -// -// Licensed under the Apache License, Version 2.0 (the "License"); -// you may not use this file except in compliance with the License. -// You may obtain a copy of the License at -// -// http://www.apache.org/licenses/LICENSE-2.0 -// -// Unless required by applicable law or agreed to in writing, software -// distributed under the License is distributed on an "AS IS" BASIS, -// WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. -// See the License for the specific language governing permissions and -// limitations under the License. - -import { CoreError } from '@classes/errors/error'; -import { CoreSubscriptions } from '@singletons/subscriptions'; -import { BehaviorSubject, Observable, of, Subscription } from 'rxjs'; -import { catchError } from 'rxjs/operators'; - -/** - * Convert to an Observable a Promise that resolves to an Observable. - * - * @param createObservable A function returning a promise that resolves to an Observable. - * @returns Observable. - */ -export function asyncObservable(createObservable: () => Promise>): Observable { - const promise = createObservable(); - - return new Observable(subscriber => { - promise - .then(observable => observable.subscribe(subscriber)) - .catch(error => subscriber.error(error)); - }); -} - -/** - * Create a Promise that resolved with the first value returned from an observable. - * This function can be removed when the app starts using rxjs v7. - * - * @param observable Observable. - * @returns Promise resolved with the first value returned. - */ -export function firstValueFrom(observable: Observable): Promise { - return new Promise((resolve, reject) => { - CoreSubscriptions.once(observable, resolve, reject, () => { - // Subscription is completed, check if we can get its value. - if (observable instanceof BehaviorSubject) { - resolve(observable.getValue()); - } - - reject(new CoreError('Couldn\'t get first value from observable because it\'s already completed')); - }); - }); -} - -/** - * Ignore errors from an observable, returning a certain value instead. - * - * @param observable Observable to ignore errors. - * @param fallback Value to return if the observer errors. - * @return Observable with ignored errors, returning the fallback result if provided. - */ -export function ignoreErrors(observable: Observable): Observable; -export function ignoreErrors(observable: Observable, fallback: Fallback): Observable; -export function ignoreErrors( - observable: Observable, - fallback?: Fallback, -): Observable { - return observable.pipe(catchError(() => of(fallback))); -} - -/** - * Get return types of a list of observables. - */ -type GetObservablesReturnTypes = { [key in keyof T]: T[key] extends Observable ? R : never }; - -/** - * Data for an observable when zipping. - */ -type ZipObservableData = { // eslint-disable-line @typescript-eslint/no-explicit-any - values: T[]; - completed: boolean; - subscription?: Subscription; -}; - -/** - * Similar to rxjs zip operator, but once an observable completes we'll continue to emit the last value as long - * as the other observables continue to emit values. - * - * @param observables Observables to zip. - * @return Observable that emits the zipped values. - */ -export function zipIncudingComplete[]>( // eslint-disable-line @typescript-eslint/no-explicit-any - ...observables: T -): Observable> { - return new Observable(subscriber => { - const observablesData: ZipObservableData[] = []; - let nextIndex = 0; - let numCompleted = 0; - let hasErrored = false; - - // Treat an emitted event. - const treatEmitted = (completed = false) => { - if (hasErrored) { - return; - } - - if (numCompleted >= observables.length) { - subscriber.complete(); - - return; - } - - // Check if any observable still doesn't have data for the index. - const notReady = observablesData.some(data => !data.completed && data.values[nextIndex] === undefined); - if (notReady) { - return; - } - - // For each observable, get the value for the next index, or last value if not present (completed). - const valueToEmit = observablesData.map(observableData => - observableData.values[nextIndex] ?? observableData.values[observableData.values.length - 1]); - - nextIndex++; - subscriber.next(> valueToEmit); - - if (completed) { - // An observable was completed, there might be other values to emit. - treatEmitted(true); - } - }; - - observables.forEach((observable, obsIndex) => { - const observableData: ZipObservableData = { - values: [], - completed: false, - }; - - observableData.subscription = observable.subscribe({ - next: (value) => { - observableData.values.push(value); - treatEmitted(); - }, - error: (error) => { - hasErrored = true; - subscriber.error(error); - }, - complete: () => { - observableData.completed = true; - numCompleted++; - treatEmitted(true); - }, - }); - - observablesData[obsIndex] = observableData; - }); - - // When unsubscribing, unsubscribe from all observables. - return () => { - observablesData.forEach(observableData => observableData.subscription?.unsubscribe()); - }; - }); -} diff --git a/src/core/utils/rxjs.ts b/src/core/utils/rxjs.ts index 509f28d82..d284b971a 100644 --- a/src/core/utils/rxjs.ts +++ b/src/core/utils/rxjs.ts @@ -13,8 +13,10 @@ // limitations under the License. import { FormControl } from '@angular/forms'; -import { Observable, OperatorFunction } from 'rxjs'; -import { filter } from 'rxjs/operators'; +import { CoreError } from '@classes/errors/error'; +import { CoreSubscriptions } from '@singletons/subscriptions'; +import { BehaviorSubject, Observable, of, OperatorFunction, Subscription } from 'rxjs'; +import { catchError, filter } from 'rxjs/operators'; /** * Create an observable that emits the current form control value. @@ -60,3 +62,149 @@ export function startWithOnSubscribed(onSubscribed: () => T): OperatorFunctio return source.subscribe(value => subscriber.next(value)); }); } + +/** + * Convert to an Observable a Promise that resolves to an Observable. + * + * @param createObservable A function returning a promise that resolves to an Observable. + * @returns Observable. + */ +export function asyncObservable(createObservable: () => Promise>): Observable { + const promise = createObservable(); + + return new Observable(subscriber => { + promise + .then(observable => observable.subscribe(subscriber)) // rxjs will automatically handle unsubscribes. + .catch(error => subscriber.error(error)); + }); +} + +/** + * Create a Promise resolved with the first value returned from an observable. The difference with toPromise is that + * this function returns the value as soon as it's emitted, it doesn't wait until the observable completes. + * This function can be removed when the app starts using rxjs v7. + * + * @param observable Observable. + * @returns Promise resolved with the first value returned. + */ +export function firstValueFrom(observable: Observable): Promise { + return new Promise((resolve, reject) => { + CoreSubscriptions.once(observable, resolve, reject, () => { + // Subscription is completed, check if we can get its value. + if (observable instanceof BehaviorSubject) { + resolve(observable.getValue()); + } + + reject(new CoreError('Couldn\'t get first value from observable because it\'s already completed')); + }); + }); +} + +/** + * Ignore errors from an observable, returning a certain value instead. + * + * @param observable Observable to ignore errors. + * @param fallback Value to return if the observer errors. + * @return Observable with ignored errors, returning the fallback result if provided. + */ +export function ignoreErrors(observable: Observable): Observable; +export function ignoreErrors(observable: Observable, fallback: Fallback): Observable; +export function ignoreErrors( + observable: Observable, + fallback?: Fallback, +): Observable { + return observable.pipe(catchError(() => of(fallback))); +} + +/** + * Get return types of a list of observables. + */ +type GetObservablesReturnTypes = { [key in keyof T]: T[key] extends Observable ? R : never }; + +/** + * Data for an observable when zipping. + */ +type ZipObservableData = { + values: T[]; + completed: boolean; + subscription?: Subscription; +}; + +/** + * Same as the built-in zip operator, but once an observable completes it'll continue to emit the last value as long + * as the other observables continue to emit values. + * + * @param observables Observables to zip. + * @return Observable that emits the zipped values. + */ +export function zipIncludingComplete[]>( + ...observables: T +): Observable> { + return new Observable(subscriber => { + const observablesData: ZipObservableData[] = []; + let nextIndex = 0; + let numCompleted = 0; + let hasErrored = false; + + // Treat an emitted event. + const treatEmitted = (completed = false) => { + if (hasErrored) { + return; + } + + if (numCompleted >= observables.length) { + subscriber.complete(); + + return; + } + + // Check if any observable still doesn't have data for the index. + const notReady = observablesData.some(data => !data.completed && data.values[nextIndex] === undefined); + if (notReady) { + return; + } + + // For each observable, get the value for the next index, or last value if not present (completed). + const valueToEmit = observablesData.map(observableData => + observableData.values[nextIndex] ?? observableData.values[observableData.values.length - 1]); + + nextIndex++; + subscriber.next(> valueToEmit); + + if (completed) { + // An observable was completed, there might be other values to emit. + treatEmitted(true); + } + }; + + observables.forEach((observable, obsIndex) => { + const observableData: ZipObservableData = { + values: [], + completed: false, + }; + + observableData.subscription = observable.subscribe({ + next: (value) => { + observableData.values.push(value); + treatEmitted(); + }, + error: (error) => { + hasErrored = true; + subscriber.error(error); + }, + complete: () => { + observableData.completed = true; + numCompleted++; + treatEmitted(true); + }, + }); + + observablesData[obsIndex] = observableData; + }); + + // When unsubscribing, unsubscribe from all observables. + return () => { + observablesData.forEach(observableData => observableData.subscription?.unsubscribe()); + }; + }); +} diff --git a/src/core/utils/tests/observables.test.ts b/src/core/utils/tests/observables.test.ts deleted file mode 100644 index 47a7a1e1c..000000000 --- a/src/core/utils/tests/observables.test.ts +++ /dev/null @@ -1,160 +0,0 @@ -// (C) Copyright 2015 Moodle Pty Ltd. -// -// Licensed under the Apache License, Version 2.0 (the "License"); -// you may not use this file except in compliance with the License. -// You may obtain a copy of the License at -// -// http://www.apache.org/licenses/LICENSE-2.0 -// -// Unless required by applicable law or agreed to in writing, software -// distributed under the License is distributed on an "AS IS" BASIS, -// WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. -// See the License for the specific language governing permissions and -// limitations under the License. - -import { BehaviorSubject, Observable, Subject } from 'rxjs'; -import { TestScheduler } from 'rxjs/testing'; -import { asyncObservable, firstValueFrom, ignoreErrors, zipIncudingComplete } from '../observables'; - -describe('Observables utility functions', () => { - - it('asyncObservable emits values', (done) => { - const subject = new Subject(); - const promise = new Promise>((resolve) => { - resolve(subject); - }); - - asyncObservable(() => promise).subscribe({ - next: (value) => { - expect(value).toEqual('foo'); - done(); - }, - }); - - // Wait for the promise callback to be called before emitting the value. - setTimeout(() => subject.next('foo')); - }); - - it('asyncObservable emits errors', (done) => { - const subject = new Subject(); - const promise = new Promise>((resolve) => { - resolve(subject); - }); - - asyncObservable(() => promise).subscribe({ - error: (value) => { - expect(value).toEqual('foo'); - done(); - }, - }); - - // Wait for the promise callback to be called before emitting the value. - setTimeout(() => subject.error('foo')); - }); - - it('asyncObservable emits complete', (done) => { - const subject = new Subject(); - const promise = new Promise>((resolve) => { - resolve(subject); - }); - - asyncObservable(() => promise).subscribe({ - complete: () => done(), - }); - - // Wait for the promise callback to be called before emitting the value. - setTimeout(() => subject.complete()); - }); - - it('asyncObservable emits error if promise is rejected', (done) => { - const promise = new Promise>((resolve, reject) => { - reject('Custom error'); - }); - - asyncObservable(() => promise).subscribe({ - error: (error) => { - expect(error).toEqual('Custom error'); - done(); - }, - }); - }); - - it('returns first value emitted by an observable', async () => { - const subject = new Subject(); - setTimeout(() => subject.next('foo'), 10); - - await expect(firstValueFrom(subject)).resolves.toEqual('foo'); - - // Check that running it again doesn't get last value, it gets the new one. - setTimeout(() => subject.next('bar'), 10); - await expect(firstValueFrom(subject)).resolves.toEqual('bar'); - - // Check we cannot get first value if a subject is already completed. - subject.complete(); - await expect(firstValueFrom(subject)).rejects.toThrow(); - - // Check that we get last value when using BehaviourSubject. - const behaviorSubject = new BehaviorSubject('baz'); - await expect(firstValueFrom(behaviorSubject)).resolves.toEqual('baz'); - - // Check we get last value even if behaviour subject is completed. - behaviorSubject.complete(); - await expect(firstValueFrom(behaviorSubject)).resolves.toEqual('baz'); - - // Check that Promise is rejected if the observable emits an error. - const errorSubject = new Subject(); - setTimeout(() => errorSubject.error('foo error'), 10); - - await expect(firstValueFrom(errorSubject)).rejects.toMatch('foo error'); - }); - - it('ignores observable errors', (done) => { - const subject = new Subject(); - - ignoreErrors(subject, 'default value').subscribe({ - next: (value) => { - expect(value).toEqual('default value'); - done(); - }, - }); - - subject.error('foo'); - }); - - it('zips observables including complete events', () => { - const scheduler = new TestScheduler((actual, expected) => { - expect(actual).toEqual(expected); - }); - - scheduler.run(({ expectObservable, cold }) => { - expectObservable(zipIncudingComplete( - cold('-a-b---|', { - a: 'A1', - b: 'A2', - }), - cold('-a----b-c--|', { - a: 'B1', - b: 'B2', - c: 'B3', - }), - cold('-a-b-c--de-----|', { - a: 'C1', - b: 'C2', - c: 'C3', - d: 'C4', - e: 'C5', - }), - )).toBe( - '-a----b-c--(de)|', - { - a: ['A1','B1','C1'], - b: ['A2','B2','C2'], - c: ['A2','B3','C3'], - d: ['A2','B3','C4'], - e: ['A2','B3','C5'], - }, - ); - }); - }); - -}); diff --git a/src/core/utils/tests/rxjs.test.ts b/src/core/utils/tests/rxjs.test.ts index 738fd389b..934b9cfb0 100644 --- a/src/core/utils/tests/rxjs.test.ts +++ b/src/core/utils/tests/rxjs.test.ts @@ -12,14 +12,23 @@ // See the License for the specific language governing permissions and // limitations under the License. -import { formControlValue, resolved, startWithOnSubscribed } from '@/core/utils/rxjs'; +import { + asyncObservable, + firstValueFrom, + formControlValue, + ignoreErrors, + resolved, + startWithOnSubscribed, + zipIncludingComplete, +} from '@/core/utils/rxjs'; import { mock } from '@/testing/utils'; import { FormControl } from '@angular/forms'; -import { of, Subject } from 'rxjs'; +import { BehaviorSubject, Observable, of, Subject } from 'rxjs'; +import { TestScheduler } from 'rxjs/testing'; describe('RXJS Utils', () => { - it('Emits filtered form values', () => { + it('formControlValue emits filtered form values', () => { // Arrange. let value = 'one'; const emited: string[] = []; @@ -48,7 +57,7 @@ describe('RXJS Utils', () => { expect(emited).toEqual(['two', 'three']); }); - it('Emits resolved values', async () => { + it('resolved emits resolved values', async () => { // Arrange. const emited: string[] = []; const promises = [ @@ -67,7 +76,7 @@ describe('RXJS Utils', () => { expect(emited).toEqual(['one', 'two', 'three']); }); - it('Adds starting values on subscription', () => { + it('startWithOnSubscribed adds starting values on subscription', () => { // Arrange. let store = 'one'; const emited: string[] = []; @@ -86,4 +95,143 @@ describe('RXJS Utils', () => { expect(emited).toEqual(['two', 'final', 'three', 'final']); }); + it('asyncObservable emits values', (done) => { + const subject = new Subject(); + const promise = new Promise>((resolve) => { + resolve(subject); + }); + + asyncObservable(() => promise).subscribe({ + next: (value) => { + expect(value).toEqual('foo'); + done(); + }, + }); + + // Wait for the promise callback to be called before emitting the value. + setTimeout(() => subject.next('foo')); + }); + + it('asyncObservable emits errors', (done) => { + const subject = new Subject(); + const promise = new Promise>((resolve) => { + resolve(subject); + }); + + asyncObservable(() => promise).subscribe({ + error: (value) => { + expect(value).toEqual('foo'); + done(); + }, + }); + + // Wait for the promise callback to be called before emitting the value. + setTimeout(() => subject.error('foo')); + }); + + it('asyncObservable emits complete', (done) => { + const subject = new Subject(); + const promise = new Promise>((resolve) => { + resolve(subject); + }); + + asyncObservable(() => promise).subscribe({ + complete: () => done(), + }); + + // Wait for the promise callback to be called before emitting the value. + setTimeout(() => subject.complete()); + }); + + it('asyncObservable emits error if promise is rejected', (done) => { + const promise = new Promise>((resolve, reject) => { + reject('Custom error'); + }); + + asyncObservable(() => promise).subscribe({ + error: (error) => { + expect(error).toEqual('Custom error'); + done(); + }, + }); + }); + + it('firstValueFrom returns first value emitted by an observable', async () => { + const subject = new Subject(); + setTimeout(() => subject.next('foo'), 10); + + await expect(firstValueFrom(subject)).resolves.toEqual('foo'); + + // Check that running it again doesn't get last value, it gets the new one. + setTimeout(() => subject.next('bar'), 10); + await expect(firstValueFrom(subject)).resolves.toEqual('bar'); + + // Check we cannot get first value if a subject is already completed. + subject.complete(); + await expect(firstValueFrom(subject)).rejects.toThrow(); + + // Check that we get last value when using BehaviourSubject. + const behaviorSubject = new BehaviorSubject('baz'); + await expect(firstValueFrom(behaviorSubject)).resolves.toEqual('baz'); + + // Check we get last value even if behaviour subject is completed. + behaviorSubject.complete(); + await expect(firstValueFrom(behaviorSubject)).resolves.toEqual('baz'); + + // Check that Promise is rejected if the observable emits an error. + const errorSubject = new Subject(); + setTimeout(() => errorSubject.error('foo error'), 10); + + await expect(firstValueFrom(errorSubject)).rejects.toMatch('foo error'); + }); + + it('ignoreErrors ignores observable errors', (done) => { + const subject = new Subject(); + + ignoreErrors(subject, 'default value').subscribe({ + next: (value) => { + expect(value).toEqual('default value'); + done(); + }, + }); + + subject.error('foo'); + }); + + it('zipIncludingComplete zips observables including complete events', () => { + const scheduler = new TestScheduler((actual, expected) => { + expect(actual).toEqual(expected); + }); + + scheduler.run(({ expectObservable, cold }) => { + expectObservable(zipIncludingComplete( + cold('-a-b---|', { + a: 'A1', + b: 'A2', + }), + cold('-a----b-c--|', { + a: 'B1', + b: 'B2', + c: 'B3', + }), + cold('-a-b-c--de-----|', { + a: 'C1', + b: 'C2', + c: 'C3', + d: 'C4', + e: 'C5', + }), + )).toBe( + '-a----b-c--(de)|', + { + a: ['A1','B1','C1'], + b: ['A2','B2','C2'], + c: ['A2','B3','C3'], + d: ['A2','B3','C4'], + e: ['A2','B3','C5'], + }, + ); + }); + }); + }); From ce9c0868198fcabf9ef96c4eac45b88e2a491e5b Mon Sep 17 00:00:00 2001 From: Dani Palou Date: Thu, 21 Jul 2022 11:04:39 +0200 Subject: [PATCH 10/14] MOBILE-3817 block: Detect changes in block input --- .../features/block/components/block/block.ts | 50 +++++-------------- .../block/components/block/core-block.html | 2 +- upgrade.txt | 1 + 3 files changed, 14 insertions(+), 39 deletions(-) diff --git a/src/core/features/block/components/block/block.ts b/src/core/features/block/components/block/block.ts index fd51ca57d..8e12855e3 100644 --- a/src/core/features/block/components/block/block.ts +++ b/src/core/features/block/components/block/block.ts @@ -12,7 +12,7 @@ // See the License for the specific language governing permissions and // limitations under the License. -import { Component, Input, OnInit, ViewChild, OnDestroy, DoCheck, KeyValueDiffers, KeyValueDiffer, Type } from '@angular/core'; +import { Component, Input, ViewChild, OnDestroy, Type, OnChanges, SimpleChanges } from '@angular/core'; import { CoreBlockDelegate } from '../../services/block-delegate'; import { CoreDynamicComponent } from '@components/dynamic-component/dynamic-component'; import { Subscription } from 'rxjs'; @@ -27,7 +27,7 @@ import type { ICoreBlockComponent } from '@features/block/classes/base-block-com templateUrl: 'core-block.html', styleUrls: ['block.scss'], }) -export class CoreBlockComponent implements OnInit, OnDestroy, DoCheck { +export class CoreBlockComponent implements OnChanges, OnDestroy { @ViewChild(CoreDynamicComponent) dynamicComponent?: CoreDynamicComponent; @@ -40,52 +40,26 @@ export class CoreBlockComponent implements OnInit, OnDestroy, DoCheck { data: Record = {}; // Data to pass to the component. class?: string; // CSS class to apply to the block. loaded = false; - blockSubscription?: Subscription; - protected differ: KeyValueDiffer; // To detect changes in the data input. - - constructor( - differs: KeyValueDiffers, - ) { - this.differ = differs.find([]).create(); - } - /** - * Component being initialized. + * @inheritdoc */ - ngOnInit(): void { - if (!this.block) { - this.loaded = true; - - return; + ngOnChanges(changes: SimpleChanges): void { + if (changes.block && this.block?.visible) { + this.updateBlock(); } - if (this.block.visible) { - // Get the data to render the block. - this.initBlock(); + if (this.data && changes.extraData) { + this.data = Object.assign(this.data, this.extraData || {}); } } /** - * Detect and act upon changes that Angular can’t or won’t detect on its own (objects and arrays). + * Get block display data and initialises or updates the block. If the block is not supported at the moment, try again if the + * available blocks are updated (because it comes from a site plugin). */ - ngDoCheck(): void { - if (this.data) { - // Check if there's any change in the extraData object. - const changes = this.differ.diff(this.extraData); - if (changes) { - this.data = Object.assign(this.data, this.extraData || {}); - } - } - } - - /** - * Get block display data and initialises the block once this is available. If the block is not - * supported at the moment, try again if the available blocks are updated (because it comes - * from a site plugin). - */ - async initBlock(): Promise { + async updateBlock(): Promise { try { const data = await CoreBlockDelegate.getBlockDisplayData(this.block, this.contextLevel, this.instanceId); @@ -97,7 +71,7 @@ export class CoreBlockComponent implements OnInit, OnDestroy, DoCheck { (): void => { this.blockSubscription?.unsubscribe(); delete this.blockSubscription; - this.initBlock(); + this.updateBlock(); }, ); diff --git a/src/core/features/block/components/block/core-block.html b/src/core/features/block/components/block/core-block.html index 4761a2dbc..663799e01 100644 --- a/src/core/features/block/components/block/core-block.html +++ b/src/core/features/block/components/block/core-block.html @@ -1,4 +1,4 @@ - + diff --git a/upgrade.txt b/upgrade.txt index 88775bda3..ad26a714e 100644 --- a/upgrade.txt +++ b/upgrade.txt @@ -6,6 +6,7 @@ information provided here is intended especially for developers. - Zoom levels changed from "normal / low / high" to " none / medium / high". - --addon-messages-* CSS3 variables have been renamed to --core-messages-* - The database constants in CoreSite (WS_CACHE_TABLE, CONFIG_TABLE, LAST_VIEWED_TABLE) and the DBRecord types have been moved to src/core/services/database. +- The component will no longer detect changes of properties inside the extraData object, it will only detect changes if the object itself changes. === 4.0.0 === From 01df501cadf31df545167bed2108978435e610a9 Mon Sep 17 00:00:00 2001 From: Dani Palou Date: Mon, 18 Jul 2022 14:07:05 +0200 Subject: [PATCH 11/14] MOBILE-3817 courses: Apply update in background to My Courses --- .../components/myoverview/myoverview.ts | 192 ++++++++++++++---- src/addons/notifications/pages/list/list.html | 2 +- src/addons/notifications/pages/list/list.scss | 2 - src/core/classes/page-load-watcher.ts | 159 +++++++++++++++ src/core/classes/page-loads-manager.ts | 141 +++++++++++++ src/core/components/components.module.ts | 3 + .../refresh-button-modal.html | 4 + .../refresh-button-modal.scss | 7 + .../refresh-button-modal.ts | 34 ++++ .../block/classes/base-block-component.ts | 37 +++- src/core/features/courses/pages/my/my.ts | 46 ++++- .../courses/services/courses-helper.ts | 5 +- src/core/singletons/object.ts | 29 +++ src/core/singletons/tests/object.test.ts | 47 +++++ src/core/utils/rxjs.ts | 14 +- src/theme/theme.base.scss | 29 ++- 16 files changed, 690 insertions(+), 61 deletions(-) create mode 100644 src/core/classes/page-load-watcher.ts create mode 100644 src/core/classes/page-loads-manager.ts create mode 100644 src/core/components/refresh-button-modal/refresh-button-modal.html create mode 100644 src/core/components/refresh-button-modal/refresh-button-modal.scss create mode 100644 src/core/components/refresh-button-modal/refresh-button-modal.ts diff --git a/src/addons/block/myoverview/components/myoverview/myoverview.ts b/src/addons/block/myoverview/components/myoverview/myoverview.ts index 53105110d..0a6a6d6ce 100644 --- a/src/addons/block/myoverview/components/myoverview/myoverview.ts +++ b/src/addons/block/myoverview/components/myoverview/myoverview.ts @@ -12,12 +12,17 @@ // See the License for the specific language governing permissions and // limitations under the License. -import { Component, OnInit, OnDestroy } from '@angular/core'; +import { Component, OnInit, OnDestroy, Optional, OnChanges, SimpleChanges } from '@angular/core'; import { CoreEventObserver, CoreEvents } from '@singletons/events'; import { CoreTimeUtils } from '@services/utils/time'; import { CoreSites, CoreSitesReadingStrategy } from '@services/sites'; -import { CoreCoursesProvider, CoreCourses, CoreCoursesMyCoursesUpdatedEventData } from '@features/courses/services/courses'; -import { CoreCoursesHelper, CoreEnrolledCourseDataWithOptions } from '@features/courses/services/courses-helper'; +import { + CoreCoursesProvider, + CoreCourses, + CoreCoursesMyCoursesUpdatedEventData, + CoreCourseSummaryData, +} from '@features/courses/services/courses'; +import { CoreCoursesHelper, CoreEnrolledCourseDataWithExtraInfoAndOptions } from '@features/courses/services/courses-helper'; import { CoreCourseHelper, CorePrefetchStatusInfo } from '@features/course/services/course-helper'; import { CoreCourseOptionsDelegate } from '@features/course/services/course-options-delegate'; import { CoreBlockBaseComponent } from '@features/block/classes/base-block-component'; @@ -28,6 +33,8 @@ import { CoreTextUtils } from '@services/utils/text'; import { AddonCourseCompletion } from '@addons/coursecompletion/services/coursecompletion'; import { IonRefresher, IonSearchbar } from '@ionic/angular'; import { CoreNavigator } from '@services/navigator'; +import { PageLoadWatcher } from '@classes/page-load-watcher'; +import { PageLoadsManager } from '@classes/page-loads-manager'; const FILTER_PRIORITY: AddonBlockMyOverviewTimeFilters[] = ['all', 'inprogress', 'future', 'past', 'favourite', 'allincludinghidden', 'hidden']; @@ -40,9 +47,9 @@ const FILTER_PRIORITY: AddonBlockMyOverviewTimeFilters[] = templateUrl: 'addon-block-myoverview.html', styleUrls: ['myoverview.scss'], }) -export class AddonBlockMyOverviewComponent extends CoreBlockBaseComponent implements OnInit, OnDestroy { +export class AddonBlockMyOverviewComponent extends CoreBlockBaseComponent implements OnInit, OnDestroy, OnChanges { - filteredCourses: CoreEnrolledCourseDataWithOptions[] = []; + filteredCourses: CoreEnrolledCourseDataWithExtraInfoAndOptions[] = []; prefetchCoursesData: CorePrefetchStatusInfo = { icon: '', @@ -84,7 +91,7 @@ export class AddonBlockMyOverviewComponent extends CoreBlockBaseComponent implem searchEnabled = false; protected currentSite!: CoreSite; - protected allCourses: CoreEnrolledCourseDataWithOptions[] = []; + protected allCourses: CoreEnrolledCourseDataWithExtraInfoAndOptions[] = []; protected prefetchIconsInitialized = false; protected isDirty = false; protected isDestroyed = false; @@ -94,15 +101,21 @@ export class AddonBlockMyOverviewComponent extends CoreBlockBaseComponent implem protected gradePeriodAfter = 0; protected gradePeriodBefore = 0; protected today = 0; + protected firstLoadWatcher?: PageLoadWatcher; + protected loadsManager: PageLoadsManager; - constructor() { + constructor(@Optional() loadsManager?: PageLoadsManager) { super('AddonBlockMyOverviewComponent'); + + this.loadsManager = loadsManager ?? new PageLoadsManager(); } /** * @inheritdoc */ async ngOnInit(): Promise { + this.firstLoadWatcher = this.loadsManager.startComponentLoad(this); + // Refresh the enabled flags if enabled. this.downloadCourseEnabled = !CoreCourses.isDownloadCourseDisabledInSite(); this.downloadCoursesEnabled = !CoreCourses.isDownloadCoursesDisabledInSite(); @@ -159,6 +172,16 @@ export class AddonBlockMyOverviewComponent extends CoreBlockBaseComponent implem }); } + /** + * @inheritdoc + */ + ngOnChanges(changes: SimpleChanges): void { + if (this.loaded && changes.block) { + // Block was re-fetched, load content. + this.reloadContent(); + } + } + /** * Refresh the data. * @@ -226,35 +249,66 @@ export class AddonBlockMyOverviewComponent extends CoreBlockBaseComponent implem * @inheritdoc */ protected async fetchContent(): Promise { - const config = this.block.configsRecord; + const loadWatcher = this.firstLoadWatcher ?? this.loadsManager.startComponentLoad(this); + this.firstLoadWatcher = undefined; - const showCategories = config?.displaycategories?.value == '1'; + await Promise.all([ + this.loadAllCourses(loadWatcher), + this.loadGracePeriod(loadWatcher), + ]); - this.allCourses = await CoreCoursesHelper.getUserCoursesWithOptions( - this.sort.selected, - undefined, - undefined, - showCategories, - { - readingStrategy: this.isDirty ? CoreSitesReadingStrategy.PREFER_NETWORK : undefined, - }, + this.loadSort(); + this.loadLayouts(this.block.configsRecord?.layouts?.value.split(',')); + + await this.loadFilters(this.block.configsRecord, loadWatcher); + + this.isDirty = false; + } + + /** + * Load all courses. + * + * @param loadWatcher To manage the requests. + * @return Promise resolved when done. + */ + protected async loadAllCourses(loadWatcher: PageLoadWatcher): Promise { + const showCategories = this.block.configsRecord?.displaycategories?.value === '1'; + + this.allCourses = await loadWatcher.watchRequest( + CoreCoursesHelper.getUserCoursesWithOptionsObservable({ + sort: this.sort.selected, + loadCategoryNames: showCategories, + readingStrategy: this.isDirty ? CoreSitesReadingStrategy.PREFER_NETWORK : loadWatcher.getReadingStrategy(), + }), + this.coursesHaveMeaningfulChanges.bind(this), ); this.hasCourses = this.allCourses.length > 0; + } + + /** + * Load grace period. + * + * @param loadWatcher To manage the requests. + * @return Promise resolved when done. + */ + protected async loadGracePeriod(loadWatcher: PageLoadWatcher): Promise { + this.hasCourses = this.allCourses.length > 0; try { - this.gradePeriodAfter = parseInt(await this.currentSite.getConfig('coursegraceperiodafter', this.isDirty), 10); - this.gradePeriodBefore = parseInt(await this.currentSite.getConfig('coursegraceperiodbefore', this.isDirty), 10); + const siteConfig = await loadWatcher.watchRequest( + this.currentSite.getConfigObservable( + undefined, + this.isDirty ? CoreSitesReadingStrategy.PREFER_NETWORK : loadWatcher.getReadingStrategy(), + ), + ); + + this.gradePeriodAfter = parseInt(siteConfig.coursegraceperiodafter, 10); + this.gradePeriodBefore = parseInt(siteConfig.coursegraceperiodbefore, 10); } catch { this.gradePeriodAfter = 0; this.gradePeriodBefore = 0; } - - this.loadSort(); - this.loadLayouts(config?.layouts?.value.split(',')); - await this.loadFilters(config); - - this.isDirty = false; } /** @@ -279,9 +333,12 @@ export class AddonBlockMyOverviewComponent extends CoreBlockBaseComponent implem * Load filters. * * @param config Block configuration. + * @param loadWatcher To manage the requests. + * @return Promise resolved when done. */ protected async loadFilters( config?: Record, + loadWatcher?: PageLoadWatcher, ): Promise { if (!this.hasCourses) { return; @@ -320,7 +377,7 @@ export class AddonBlockMyOverviewComponent extends CoreBlockBaseComponent implem this.saveFilters('all'); } - await this.filterCourses(); + await this.filterCourses(loadWatcher); } /** @@ -369,18 +426,14 @@ export class AddonBlockMyOverviewComponent extends CoreBlockBaseComponent implem protected async refreshCourseList(data: CoreCoursesMyCoursesUpdatedEventData): Promise { if (data.action == CoreCoursesProvider.ACTION_ENROL) { // Always update if user enrolled in a course. - this.loaded = false; - - return this.refreshContent(); + return this.refreshContent(true); } const course = this.allCourses.find((course) => course.id == data.courseId); if (data.action == CoreCoursesProvider.ACTION_STATE_CHANGED) { if (!course) { // Not found, use WS update. - this.loaded = false; - - return this.refreshContent(); + return this.refreshContent(true); } if (data.state == CoreCoursesProvider.STATE_FAVOURITE) { @@ -398,9 +451,7 @@ export class AddonBlockMyOverviewComponent extends CoreBlockBaseComponent implem if (data.action == CoreCoursesProvider.ACTION_VIEW && data.courseId != CoreSites.getCurrentSiteHomeId()) { if (!course) { // Not found, use WS update. - this.loaded = false; - - return this.refreshContent(); + return this.refreshContent(true); } course.lastaccess = CoreTimeUtils.timestamp(); @@ -457,8 +508,11 @@ export class AddonBlockMyOverviewComponent extends CoreBlockBaseComponent implem /** * Set selected courses filter. + * + * @param loadWatcher To manage the requests. + * @return Promise resolved when done. */ - protected async filterCourses(): Promise { + protected async filterCourses(loadWatcher?: PageLoadWatcher): Promise { let timeFilter = this.filters.timeFilterSelected; this.filteredCourses = this.allCourses; @@ -473,7 +527,15 @@ export class AddonBlockMyOverviewComponent extends CoreBlockBaseComponent implem this.loaded = false; try { - const courses = await CoreCourses.getEnrolledCoursesByCustomField(customFilterName, customFilterValue); + const courses = loadWatcher ? + await loadWatcher.watchRequest( + CoreCourses.getEnrolledCoursesByCustomFieldObservable(customFilterName, customFilterValue, { + readingStrategy: loadWatcher.getReadingStrategy(), + }), + this.customFilterCoursesHaveMeaningfulChanges.bind(this), + ) + : + await CoreCourses.getEnrolledCoursesByCustomField(customFilterName, customFilterValue); // Get the courses information from allincludinghidden to get the max info about the course. const courseIds = courses.map((course) => course.id); @@ -642,6 +704,62 @@ export class AddonBlockMyOverviewComponent extends CoreBlockBaseComponent implem CoreNavigator.navigateToSitePath('courses/list', { params : { mode: 'search' } }); } + /** + * Compare if the WS data has meaningful changes for the user. + * + * @param previousCourses Previous courses. + * @param newCourses New courses. + * @return Whether it has meaningful changes. + */ + protected coursesHaveMeaningfulChanges( + previousCourses: CoreEnrolledCourseDataWithExtraInfoAndOptions[], + newCourses: CoreEnrolledCourseDataWithExtraInfoAndOptions[], + ): boolean { + if (previousCourses.length !== newCourses.length) { + return true; + } + + previousCourses = Array.from(previousCourses) + .sort((a, b) => a.fullname.toLowerCase().localeCompare(b.fullname.toLowerCase())); + newCourses = Array.from(newCourses).sort((a, b) => a.fullname.toLowerCase().localeCompare(b.fullname.toLowerCase())); + + for (let i = 0; i < previousCourses.length; i++) { + const prevCourse = previousCourses[i]; + const newCourse = newCourses[i]; + + if ( + prevCourse.progress !== newCourse.progress || + prevCourse.categoryname !== newCourse.categoryname || + (prevCourse.displayname ?? prevCourse.fullname) !== (newCourse.displayname ?? newCourse.fullname) + ) { + return true; + } + } + + return false; + } + + /** + * Compare if the WS data has meaningful changes for the user. + * + * @param previousCourses Previous courses. + * @param newCourses New courses. + * @return Whether it has meaningful changes. + */ + protected customFilterCoursesHaveMeaningfulChanges( + previousCourses: CoreCourseSummaryData[], + newCourses: CoreCourseSummaryData[], + ): boolean { + if (previousCourses.length !== newCourses.length) { + return true; + } + + const previousIds = previousCourses.map(course => course.id).sort(); + const newIds = newCourses.map(course => course.id).sort(); + + return previousIds.some((previousId, index) => previousId !== newIds[index]); + } + /** * @inheritdoc */ diff --git a/src/addons/notifications/pages/list/list.html b/src/addons/notifications/pages/list/list.html index 1a2cd5589..c0ccfd14d 100644 --- a/src/addons/notifications/pages/list/list.html +++ b/src/addons/notifications/pages/list/list.html @@ -71,7 +71,7 @@
+ color="info" class="clickable fab-chip" (click)="markAllNotificationsAsRead()"> diff --git a/src/addons/notifications/pages/list/list.scss b/src/addons/notifications/pages/list/list.scss index e58fa6b12..28bc96fc3 100644 --- a/src/addons/notifications/pages/list/list.scss +++ b/src/addons/notifications/pages/list/list.scss @@ -56,9 +56,7 @@ ion-item { pointer-events: none; ion-chip.ion-color { - pointer-events: all; margin: 0 auto; - box-shadow: 0 2px 4px rgba(0, 0, 0, .4); ion-spinner { width: 16px; diff --git a/src/core/classes/page-load-watcher.ts b/src/core/classes/page-load-watcher.ts new file mode 100644 index 000000000..a76594db2 --- /dev/null +++ b/src/core/classes/page-load-watcher.ts @@ -0,0 +1,159 @@ +// (C) Copyright 2015 Moodle Pty Ltd. +// +// Licensed under the Apache License, Version 2.0 (the "License"); +// you may not use this file except in compliance with the License. +// You may obtain a copy of the License at +// +// http://www.apache.org/licenses/LICENSE-2.0 +// +// Unless required by applicable law or agreed to in writing, software +// distributed under the License is distributed on an "AS IS" BASIS, +// WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. +// See the License for the specific language governing permissions and +// limitations under the License. + +import { CoreSitesReadingStrategy } from '@services/sites'; +import { CoreUtils } from '@services/utils/utils'; +import { Subscription } from 'rxjs'; +import { AsyncComponent } from './async-component'; +import { PageLoadsManager } from './page-loads-manager'; +import { CorePromisedValue } from './promised-value'; +import { WSObservable } from './site'; + +/** + * Class to watch requests from a page load (including requests from page sub-components). + */ +export class PageLoadWatcher { + + protected hasChanges = false; + protected ongoingRequests = 0; + protected components = new Set(); + protected loadedTimeout?: number; + + constructor( + protected loadsManager: PageLoadsManager, + protected updateInBackground: boolean, + ) { } + + /** + * Whether this load watcher can update data in background. + * + * @return Whether this load watcher can update data in background. + */ + canUpdateInBackground(): boolean { + return this.updateInBackground; + } + + /** + * Whether this load watcher had meaningful changes received in background. + * + * @return Whether this load watcher had meaningful changes received in background. + */ + hasMeaningfulChanges(): boolean { + return this.hasChanges; + } + + /** + * Set has meaningful changes to true. + */ + markMeaningfulChanges(): void { + this.hasChanges = true; + } + + /** + * Watch a component, waiting for it to be ready. + * + * @param component Component instance. + */ + async watchComponent(component: AsyncComponent): Promise { + this.components.add(component); + clearTimeout(this.loadedTimeout); + + try { + await component.ready(); + } finally { + this.components.delete(component); + this.checkHasLoaded(); + } + } + + /** + * Get the reading strategy to use. + * + * @return Reading strategy to use. + */ + getReadingStrategy(): CoreSitesReadingStrategy | undefined { + return this.updateInBackground ? CoreSitesReadingStrategy.STALE_WHILE_REVALIDATE : undefined; + } + + /** + * Watch a WS request, handling the different values it can return, calling the hasMeaningfulChanges callback if needed to + * detect if there are new meaningful changes in the page load, and completing the page load when all requests have + * finished and all components are ready. + * + * @param observable Observable of the request. + * @param hasMeaningfulChanges Callback to check if there are meaningful changes if data was updated in background. + * @return First value of the observable. + */ + watchRequest( + observable: WSObservable, + hasMeaningfulChanges?: (previousValue: T, newValue: T) => boolean, + ): Promise { + const promisedValue = new CorePromisedValue(); + let subscription: Subscription | null = null; + let firstValue: T | undefined; + this.ongoingRequests++; + clearTimeout(this.loadedTimeout); + + const complete = async () => { + this.ongoingRequests--; + this.checkHasLoaded(); + + // Subscription variable might not be set because the observable completed immediately. Wait for next tick. + await CoreUtils.nextTick(); + subscription?.unsubscribe(); + }; + + subscription = observable.subscribe({ + next: value => { + if (!firstValue) { + firstValue = value; + promisedValue.resolve(value); + + return; + } + + // Second value, it means data was updated in background. Compare data. + if (hasMeaningfulChanges?.(firstValue, value)) { + this.hasChanges = true; + } + }, + error: (error) => { + promisedValue.reject(error); + complete(); + }, + complete: () => complete(), + }); + + return promisedValue; + } + + /** + * Check if the load has finished. + */ + protected checkHasLoaded(): void { + if (this.ongoingRequests !== 0 || this.components.size !== 0) { + // Load not finished. + return; + } + + // It seems load has finished. Wait to make sure no new component has been rendered and started loading. + // If a new component or a new request starts the timeout will be cancelled, no need to double check it. + clearTimeout(this.loadedTimeout); + this.loadedTimeout = window.setTimeout(() => { + // Loading finished. + this.loadsManager.onPageLoaded(this); + }, 100); + } + +} diff --git a/src/core/classes/page-loads-manager.ts b/src/core/classes/page-loads-manager.ts new file mode 100644 index 000000000..0d1bf7225 --- /dev/null +++ b/src/core/classes/page-loads-manager.ts @@ -0,0 +1,141 @@ +// (C) Copyright 2015 Moodle Pty Ltd. +// +// Licensed under the Apache License, Version 2.0 (the "License"); +// you may not use this file except in compliance with the License. +// You may obtain a copy of the License at +// +// http://www.apache.org/licenses/LICENSE-2.0 +// +// Unless required by applicable law or agreed to in writing, software +// distributed under the License is distributed on an "AS IS" BASIS, +// WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. +// See the License for the specific language governing permissions and +// limitations under the License. + +import { CoreRefreshButtonModalComponent } from '@components/refresh-button-modal/refresh-button-modal'; +import { CoreNavigator } from '@services/navigator'; +import { CoreDomUtils } from '@services/utils/dom'; +import { Subject } from 'rxjs'; +import { AsyncComponent } from './async-component'; +import { PageLoadWatcher } from './page-load-watcher'; + +/** + * Class to manage requests in a page and its components. + */ +export class PageLoadsManager { + + onRefreshPage = new Subject(); + + protected initialPath?: string; + protected currentLoadWatcher?: PageLoadWatcher; + protected ongoingLoadWatchers = new Set(); + + /** + * Start a page load, creating a new load watcher and watching the page. + * + * @param page Page instance. + * @param staleWhileRevalidate Whether to use stale while revalidate strategy. + * @return Load watcher to use. + */ + startPageLoad(page: AsyncComponent, staleWhileRevalidate: boolean): PageLoadWatcher { + this.initialPath = this.initialPath ?? CoreNavigator.getCurrentPath(); + this.currentLoadWatcher = new PageLoadWatcher(this, staleWhileRevalidate); + this.ongoingLoadWatchers.add(this.currentLoadWatcher); + + this.currentLoadWatcher.watchComponent(page); + + return this.currentLoadWatcher; + } + + /** + * Start a component load, adding it to currrent load watcher (if it exists) and watching the component. + * + * @param component Component instance. + * @return Load watcher to use. + */ + startComponentLoad(component: AsyncComponent): PageLoadWatcher { + // If a component is loading data without the page loading data, probably the component is reloading/refreshing. + // In that case, create a load watcher instance but don't store it in currentLoadWatcher because it's not a page load. + const loadWatcher = this.currentLoadWatcher ?? new PageLoadWatcher(this, false); + + loadWatcher.watchComponent(component); + + return loadWatcher; + } + + /** + * A load has finished, remove its watcher from ongoing watchers and notify if needed. + * + * @param loadWatcher Load watcher related to the load that finished. + */ + onPageLoaded(loadWatcher: PageLoadWatcher): void { + if (!this.ongoingLoadWatchers.has(loadWatcher)) { + // Watcher not in list, it probably finished already. + return; + } + + this.removeLoadWatcher(loadWatcher); + + if (!loadWatcher.hasMeaningfulChanges()) { + // No need to notify. + return; + } + + // Check if there is another ongoing load watcher using update in background. + // If so, wait for the other one to finish before notifying to prevent being notified twice. + const ongoingLoadWatcher = this.getAnotherOngoingUpdateInBackgroundWatcher(loadWatcher); + if (ongoingLoadWatcher) { + ongoingLoadWatcher.markMeaningfulChanges(); + + return; + } + + if (this.initialPath === CoreNavigator.getCurrentPath()) { + // User hasn't changed page, notify them. + this.notifyUser(); + } else { + // User left the page, just update the data. + this.onRefreshPage.next(); + } + } + + /** + * Get an ongoing load watcher that supports updating in background and is not the one passed as a parameter. + * + * @param loadWatcher Load watcher to ignore. + * @return Ongoing load watcher, undefined if none found. + */ + protected getAnotherOngoingUpdateInBackgroundWatcher(loadWatcher: PageLoadWatcher): PageLoadWatcher | undefined { + for (const ongoingLoadWatcher of this.ongoingLoadWatchers) { + if (ongoingLoadWatcher.canUpdateInBackground() && loadWatcher !== ongoingLoadWatcher) { + return ongoingLoadWatcher; + } + } + } + + /** + * Remove a load watcher from the list. + * + * @param loadWatcher Load watcher to remove. + */ + protected removeLoadWatcher(loadWatcher: PageLoadWatcher): void { + this.ongoingLoadWatchers.delete(loadWatcher); + if (loadWatcher === this.currentLoadWatcher) { + delete this.currentLoadWatcher; + } + } + + /** + * Notify the user, asking him if he wants to update the data. + */ + protected async notifyUser(): Promise { + await CoreDomUtils.openModal({ + component: CoreRefreshButtonModalComponent, + cssClass: 'core-modal-no-background', + closeOnNavigate: true, + }); + + this.onRefreshPage.next(); + } + +} diff --git a/src/core/components/components.module.ts b/src/core/components/components.module.ts index cc05dd2b4..846aee4e6 100644 --- a/src/core/components/components.module.ts +++ b/src/core/components/components.module.ts @@ -63,6 +63,7 @@ import { CoreSwipeSlidesComponent } from './swipe-slides/swipe-slides'; import { CoreSwipeNavigationTourComponent } from './swipe-navigation-tour/swipe-navigation-tour'; import { CoreMessageComponent } from './message/message'; import { CoreGroupSelectorComponent } from './group-selector/group-selector'; +import { CoreRefreshButtonModalComponent } from './refresh-button-modal/refresh-button-modal'; @NgModule({ declarations: [ @@ -108,6 +109,7 @@ import { CoreGroupSelectorComponent } from './group-selector/group-selector'; CoreSpacerComponent, CoreHorizontalScrollControlsComponent, CoreSwipeNavigationTourComponent, + CoreRefreshButtonModalComponent, ], imports: [ CommonModule, @@ -160,6 +162,7 @@ import { CoreGroupSelectorComponent } from './group-selector/group-selector'; CoreSpacerComponent, CoreHorizontalScrollControlsComponent, CoreSwipeNavigationTourComponent, + CoreRefreshButtonModalComponent, ], }) export class CoreComponentsModule {} diff --git a/src/core/components/refresh-button-modal/refresh-button-modal.html b/src/core/components/refresh-button-modal/refresh-button-modal.html new file mode 100644 index 000000000..c18d9761e --- /dev/null +++ b/src/core/components/refresh-button-modal/refresh-button-modal.html @@ -0,0 +1,4 @@ + + + {{ 'core.refresh' | translate }} + diff --git a/src/core/components/refresh-button-modal/refresh-button-modal.scss b/src/core/components/refresh-button-modal/refresh-button-modal.scss new file mode 100644 index 000000000..5541fba16 --- /dev/null +++ b/src/core/components/refresh-button-modal/refresh-button-modal.scss @@ -0,0 +1,7 @@ +@import "~theme/globals"; + +:host { + ion-chip { + @include margin(auto, auto, calc(12px + var(--bottom-tabs-size, 0px)), auto); + } +} diff --git a/src/core/components/refresh-button-modal/refresh-button-modal.ts b/src/core/components/refresh-button-modal/refresh-button-modal.ts new file mode 100644 index 000000000..af0c7078a --- /dev/null +++ b/src/core/components/refresh-button-modal/refresh-button-modal.ts @@ -0,0 +1,34 @@ +// (C) Copyright 2015 Moodle Pty Ltd. +// +// Licensed under the Apache License, Version 2.0 (the "License"); +// you may not use this file except in compliance with the License. +// You may obtain a copy of the License at +// +// http://www.apache.org/licenses/LICENSE-2.0 +// +// Unless required by applicable law or agreed to in writing, software +// distributed under the License is distributed on an "AS IS" BASIS, +// WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. +// See the License for the specific language governing permissions and +// limitations under the License. + +import { Component } from '@angular/core'; +import { ModalController } from '@singletons'; + +/** + * Modal that displays a refresh button. + */ +@Component({ + templateUrl: 'refresh-button-modal.html', + styleUrls: ['refresh-button-modal.scss'], +}) +export class CoreRefreshButtonModalComponent { + + /** + * Close modal. + */ + closeModal(): void { + ModalController.dismiss(); + } + +} diff --git a/src/core/features/block/classes/base-block-component.ts b/src/core/features/block/classes/base-block-component.ts index 58d3878ab..d31b7cac7 100644 --- a/src/core/features/block/classes/base-block-component.ts +++ b/src/core/features/block/classes/base-block-component.ts @@ -21,6 +21,8 @@ import { CoreCourseBlock } from '../../course/services/course'; import { Params } from '@angular/router'; import { ContextLevel } from '@/core/constants'; import { CoreNavigationOptions } from '@services/navigator'; +import { AsyncComponent } from '@classes/async-component'; +import { CorePromisedValue } from '@classes/promised-value'; /** * Template class to easily create components for blocks. @@ -28,7 +30,7 @@ import { CoreNavigationOptions } from '@services/navigator'; @Component({ template: '', }) -export abstract class CoreBlockBaseComponent implements OnInit, ICoreBlockComponent { +export abstract class CoreBlockBaseComponent implements OnInit, ICoreBlockComponent, AsyncComponent { @Input() title!: string; // The block title. @Input() block!: CoreCourseBlock; // The block to render. @@ -38,8 +40,9 @@ export abstract class CoreBlockBaseComponent implements OnInit, ICoreBlockCompon @Input() linkParams?: Params; // Link params to go when clicked. @Input() navOptions?: CoreNavigationOptions; // Navigation options. - loaded = false; // If the component has been loaded. + loaded = false; // If false, the UI should display a loading. protected fetchContentDefaultError = ''; // Default error to show when loading contents. + protected onReadyPromise = new CorePromisedValue(); protected logger: CoreLogger; @@ -65,9 +68,14 @@ export abstract class CoreBlockBaseComponent implements OnInit, ICoreBlockCompon /** * Perform the refresh content function. * + * @param showLoading Whether to show loading. * @return Resolved when done. */ - protected async refreshContent(): Promise { + protected async refreshContent(showLoading?: boolean): Promise { + if (showLoading) { + this.loaded = false; + } + // Wrap the call in a try/catch so the workflow isn't interrupted if an error occurs. try { await this.invalidateContent(); @@ -102,6 +110,7 @@ export abstract class CoreBlockBaseComponent implements OnInit, ICoreBlockCompon } this.loaded = true; + this.onReadyPromise.resolve(); } /** @@ -113,6 +122,28 @@ export abstract class CoreBlockBaseComponent implements OnInit, ICoreBlockCompon return; } + /** + * Reload content without invalidating data. + * + * @return Promise resolved when done. + */ + async reloadContent(): Promise { + if (!this.loaded) { + // Content being loaded, don't do anything. + return; + } + + this.loaded = false; + await this.loadContent(); + } + + /** + * @inheritdoc + */ + async ready(): Promise { + return await this.onReadyPromise; + } + } /** diff --git a/src/core/features/courses/pages/my/my.ts b/src/core/features/courses/pages/my/my.ts index 5bc3723d3..3f8b321a0 100644 --- a/src/core/features/courses/pages/my/my.ts +++ b/src/core/features/courses/pages/my/my.ts @@ -14,6 +14,9 @@ import { AddonBlockMyOverviewComponent } from '@addons/block/myoverview/components/myoverview/myoverview'; import { Component, OnDestroy, OnInit, ViewChild } from '@angular/core'; +import { AsyncComponent } from '@classes/async-component'; +import { PageLoadsManager } from '@classes/page-loads-manager'; +import { CorePromisedValue } from '@classes/promised-value'; import { CoreBlockComponent } from '@features/block/components/block/block'; import { CoreCourseBlock } from '@features/course/services/course'; import { CoreCoursesDashboard, CoreCoursesDashboardProvider } from '@features/courses/services/dashboard'; @@ -23,6 +26,7 @@ import { CoreSites } from '@services/sites'; import { CoreDomUtils } from '@services/utils/dom'; import { CoreUtils } from '@services/utils/utils'; import { CoreEventObserver, CoreEvents } from '@singletons/events'; +import { Subscription } from 'rxjs'; import { CoreCourses } from '../../services/courses'; /** @@ -32,8 +36,12 @@ import { CoreCourses } from '../../services/courses'; selector: 'page-core-courses-my', templateUrl: 'my.html', styleUrls: ['my.scss'], + providers: [{ + provide: PageLoadsManager, + useClass: PageLoadsManager, + }], }) -export class CoreCoursesMyCoursesPage implements OnInit, OnDestroy { +export class CoreCoursesMyCoursesPage implements OnInit, OnDestroy, AsyncComponent { @ViewChild(CoreBlockComponent) block!: CoreBlockComponent; @@ -47,8 +55,10 @@ export class CoreCoursesMyCoursesPage implements OnInit, OnDestroy { hasSideBlocks = false; protected updateSiteObserver: CoreEventObserver; + protected onReadyPromise = new CorePromisedValue(); + protected loadsManagerSubscription: Subscription; - constructor() { + constructor(protected loadsManager: PageLoadsManager) { // Refresh the enabled flags if site is updated. this.updateSiteObserver = CoreEvents.on(CoreEvents.SITE_UPDATED, () => { this.downloadCoursesEnabled = !CoreCourses.isDownloadCoursesDisabledInSite(); @@ -57,6 +67,11 @@ export class CoreCoursesMyCoursesPage implements OnInit, OnDestroy { }, CoreSites.getCurrentSiteId()); this.userId = CoreSites.getCurrentSiteUserId(); + + this.loadsManagerSubscription = this.loadsManager.onRefreshPage.subscribe(() => { + this.loaded = false; + this.loadContent(); + }); } /** @@ -70,19 +85,27 @@ export class CoreCoursesMyCoursesPage implements OnInit, OnDestroy { this.loadSiteName(); - this.loadContent(); + this.loadContent(true); } /** - * Load my overview block instance. + * Load data. + * + * @param firstLoad Whether it's the first load. */ - protected async loadContent(): Promise { + protected async loadContent(firstLoad = false): Promise { + const loadWatcher = this.loadsManager.startPageLoad(this, !!firstLoad); const available = await CoreCoursesDashboard.isAvailable(); const disabled = await CoreCourses.isMyCoursesDisabled(); if (available && !disabled) { try { - const blocks = await CoreCoursesDashboard.getDashboardBlocks(undefined, undefined, this.myPageCourses); + const blocks = await loadWatcher.watchRequest( + CoreCoursesDashboard.getDashboardBlocksObservable({ + myPage: this.myPageCourses, + readingStrategy: loadWatcher.getReadingStrategy(), + }), + ); // My overview block should always be in main blocks, but check side blocks too just in case. this.loadedBlock = blocks.mainBlocks.concat(blocks.sideBlocks).find((block) => block.name == 'myoverview'); @@ -106,6 +129,7 @@ export class CoreCoursesMyCoursesPage implements OnInit, OnDestroy { } this.loaded = true; + this.onReadyPromise.resolve(); } /** @@ -138,7 +162,7 @@ export class CoreCoursesMyCoursesPage implements OnInit, OnDestroy { // Invalidate the blocks. if (this.myOverviewBlock) { - promises.push(CoreUtils.ignoreErrors(this.myOverviewBlock.doRefresh())); + promises.push(CoreUtils.ignoreErrors(this.myOverviewBlock.invalidateContent())); } Promise.all(promises).finally(() => { @@ -153,6 +177,14 @@ export class CoreCoursesMyCoursesPage implements OnInit, OnDestroy { */ ngOnDestroy(): void { this.updateSiteObserver?.off(); + this.loadsManagerSubscription.unsubscribe(); + } + + /** + * @inheritdoc + */ + async ready(): Promise { + return await this.onReadyPromise; } } diff --git a/src/core/features/courses/services/courses-helper.ts b/src/core/features/courses/services/courses-helper.ts index a4231cec5..4e81eac5c 100644 --- a/src/core/features/courses/services/courses-helper.ts +++ b/src/core/features/courses/services/courses-helper.ts @@ -227,7 +227,7 @@ export class CoreCoursesHelperProvider { filter?: string, loadCategoryNames: boolean = false, options: CoreSitesCommonWSOptions = {}, - ): Promise { + ): Promise { return firstValueFrom(this.getUserCoursesWithOptionsObservable({ sort, slice, @@ -245,7 +245,8 @@ export class CoreCoursesHelperProvider { */ getUserCoursesWithOptionsObservable( options: CoreCoursesGetWithOptionsOptions = {}, - ): Observable { + ): Observable { + return CoreCourses.getUserCoursesObservable(options).pipe( chainRequests(options.readingStrategy, (courses, newReadingStrategy) => { if (courses.length <= 0) { diff --git a/src/core/singletons/object.ts b/src/core/singletons/object.ts index 5e5571451..a26584d60 100644 --- a/src/core/singletons/object.ts +++ b/src/core/singletons/object.ts @@ -65,6 +65,35 @@ export class CoreObject { return Object.keys(object).length === 0; } + /** + * Return an object including only certain keys. + * + * @param obj Object. + * @param keysOrRegex If array is supplied, keys to include. Otherwise, regular expression used to filter keys. + * @return New object with only the specified keys. + */ + static only(obj: T, keys: K[]): Pick; + static only(obj: T, regex: RegExp): Partial; + static only(obj: T, keysOrRegex: K[] | RegExp): Pick | Partial { + const newObject: Partial = {}; + + if (Array.isArray(keysOrRegex)) { + for (const key of keysOrRegex) { + newObject[key] = obj[key]; + } + } else { + const originalKeys = Object.keys(obj); + + for (const key of originalKeys) { + if (key.match(keysOrRegex)) { + newObject[key] = obj[key]; + } + } + } + + return newObject; + } + /** * Create a new object without the specified keys. * diff --git a/src/core/singletons/tests/object.test.ts b/src/core/singletons/tests/object.test.ts index a3b5409b7..c0ce0d302 100644 --- a/src/core/singletons/tests/object.test.ts +++ b/src/core/singletons/tests/object.test.ts @@ -99,6 +99,53 @@ describe('CoreObject singleton', () => { expect(CoreObject.isEmpty({ foo: 1 })).toEqual(false); }); + it('creates a copy of an object with certain properties (using a list)', () => { + const originalObject = { + foo: 1, + bar: 2, + baz: 3, + }; + + expect(CoreObject.only(originalObject, [])).toEqual({}); + expect(CoreObject.only(originalObject, ['foo'])).toEqual({ + foo: 1, + }); + expect(CoreObject.only(originalObject, ['foo', 'baz'])).toEqual({ + foo: 1, + baz: 3, + }); + expect(CoreObject.only(originalObject, ['foo', 'bar', 'baz'])).toEqual(originalObject); + expect(originalObject).toEqual({ + foo: 1, + bar: 2, + baz: 3, + }); + }); + + it('creates a copy of an object with certain properties (using a regular expression)', () => { + const originalObject = { + foo: 1, + bar: 2, + baz: 3, + }; + + expect(CoreObject.only(originalObject, /.*/)).toEqual(originalObject); + expect(CoreObject.only(originalObject, /^ba.*/)).toEqual({ + bar: 2, + baz: 3, + }); + expect(CoreObject.only(originalObject, /(foo|bar)/)).toEqual({ + foo: 1, + bar: 2, + }); + expect(CoreObject.only(originalObject, /notfound/)).toEqual({}); + expect(originalObject).toEqual({ + foo: 1, + bar: 2, + baz: 3, + }); + }); + it('creates a copy of an object without certain properties', () => { const originalObject = { foo: 1, diff --git a/src/core/utils/rxjs.ts b/src/core/utils/rxjs.ts index d284b971a..463b9b332 100644 --- a/src/core/utils/rxjs.ts +++ b/src/core/utils/rxjs.ts @@ -126,6 +126,7 @@ type GetObservablesReturnTypes = { [key in keyof T]: T[key] extends Observabl */ type ZipObservableData = { values: T[]; + hasValueForIndex: boolean[]; completed: boolean; subscription?: Subscription; }; @@ -159,7 +160,7 @@ export function zipIncludingComplete[]>( } // Check if any observable still doesn't have data for the index. - const notReady = observablesData.some(data => !data.completed && data.values[nextIndex] === undefined); + const notReady = observablesData.some(data => !data.completed && !data.hasValueForIndex[nextIndex]); if (notReady) { return; } @@ -177,15 +178,22 @@ export function zipIncludingComplete[]>( } }; + // Before subscribing, initialize the data for all observables. observables.forEach((observable, obsIndex) => { - const observableData: ZipObservableData = { + observablesData[obsIndex] = { values: [], + hasValueForIndex: [], completed: false, }; + }); + + observables.forEach((observable, obsIndex) => { + const observableData = observablesData[obsIndex]; observableData.subscription = observable.subscribe({ next: (value) => { observableData.values.push(value); + observableData.hasValueForIndex.push(true); treatEmitted(); }, error: (error) => { @@ -198,8 +206,6 @@ export function zipIncludingComplete[]>( treatEmitted(true); }, }); - - observablesData[obsIndex] = observableData; }); // When unsubscribing, unsubscribe from all observables. diff --git a/src/theme/theme.base.scss b/src/theme/theme.base.scss index 9e6103704..4360e0529 100644 --- a/src/theme/theme.base.scss +++ b/src/theme/theme.base.scss @@ -1116,17 +1116,23 @@ ion-button.chip { } ion-chip { - line-height: 1.1; - font-size: 12px; padding: 4px 8px; - min-height: 24px; height: auto; - // Chips are not currently clickable. - &.ion-activatable { + // Chips are not currently clickable, only if specified explicitly. + &.ion-activatable:not(.clickable) { cursor: auto; pointer-events: none; } + &.clickable { + cursor: pointer; + pointer-events: auto; + } + + &.fab-chip { + padding: 8px 12px; + box-shadow: 0 2px 4px rgba(0, 0, 0, .4); + } &.ion-color { background: var(--ion-color-tint); @@ -1135,6 +1141,10 @@ ion-chip { border-color: var(--ion-color-base); color: var(--ion-color-base); } + &.fab-chip { + background: var(--ion-color); + color: var(--ion-color-contrast); + } &.ion-color-light, &.ion-color-medium, @@ -1739,3 +1749,12 @@ ion-header.no-title { video::-webkit-media-text-track-display { white-space: normal !important; } + +ion-modal.core-modal-no-background { + --background: transparent; + pointer-events: none; + + ion-backdrop { + display: none; + } +} From 979e9951666e202f0830c075ef0fd1f0463967f7 Mon Sep 17 00:00:00 2001 From: Dani Palou Date: Mon, 25 Jul 2022 10:39:59 +0200 Subject: [PATCH 12/14] MOBILE-3817 filter: Apply update in background to filters --- src/core/classes/site.ts | 2 +- src/core/features/course/services/course.ts | 184 +++++++++++------- .../features/filter/services/filter-helper.ts | 16 +- src/core/features/filter/services/filter.ts | 8 +- src/core/services/sites.ts | 4 +- 5 files changed, 138 insertions(+), 76 deletions(-) diff --git a/src/core/classes/site.ts b/src/core/classes/site.ts index ec047eb8a..dfbeef584 100644 --- a/src/core/classes/site.ts +++ b/src/core/classes/site.ts @@ -2409,7 +2409,7 @@ export function chainRequests>( return source.subscribe({ next: async (value) => { - if (readingStrategy !== CoreSitesReadingStrategy.UPDATE_IN_BACKGROUND) { + if (readingStrategy !== CoreSitesReadingStrategy.STALE_WHILE_REVALIDATE) { // Just use same strategy. subscriber.next({ data: value, readingStrategy }); diff --git a/src/core/features/course/services/course.ts b/src/core/features/course/services/course.ts index f5a283831..d382d49be 100644 --- a/src/core/features/course/services/course.ts +++ b/src/core/features/course/services/course.ts @@ -54,6 +54,9 @@ import { CoreDatabaseCachingStrategy } from '@classes/database/database-table-pr import { SQLiteDB } from '@classes/sqlitedb'; import { CorePlatform } from '@services/platform'; import { CoreTime } from '@singletons/time'; +import { Observable } from 'rxjs'; +import { asyncObservable, firstValueFrom } from '@/core/utils/rxjs'; +import { map } from 'rxjs/operators'; const ROOT_CACHE_KEY = 'mmCourse:'; @@ -402,19 +405,36 @@ export class CoreCourseProvider { * @return Promise resolved with the list of blocks. * @since 3.7 */ - async getCourseBlocks(courseId: number, siteId?: string): Promise { - const site = await CoreSites.getSite(siteId); - const params: CoreBlockGetCourseBlocksWSParams = { - courseid: courseId, - returncontents: true, - }; - const preSets: CoreSiteWSPreSets = { - cacheKey: this.getCourseBlocksCacheKey(courseId), - updateFrequency: CoreSite.FREQUENCY_RARELY, - }; - const result = await site.read('core_block_get_course_blocks', params, preSets); + getCourseBlocks(courseId: number, siteId?: string): Promise { + return firstValueFrom(this.getCourseBlocksObservable(courseId, { siteId })); + } - return result.blocks || []; + /** + * Get course blocks. + * + * @param courseId Course ID. + * @param options Options. + * @return Observable that returns the blocks. + * @since 3.7 + */ + getCourseBlocksObservable(courseId: number, options: CoreSitesCommonWSOptions = {}): Observable { + return asyncObservable(async () => { + const site = await CoreSites.getSite(options.siteId); + + const params: CoreBlockGetCourseBlocksWSParams = { + courseid: courseId, + returncontents: true, + }; + const preSets: CoreSiteWSPreSets = { + cacheKey: this.getCourseBlocksCacheKey(courseId), + updateFrequency: CoreSite.FREQUENCY_RARELY, + ...CoreSites.getReadingStrategyPreSets(options.readingStrategy), + }; + + return site.readObservable('core_block_get_course_blocks', params, preSets).pipe( + map(result => result.blocks), + ); + }); } /** @@ -908,7 +928,7 @@ export class CoreCourseProvider { * @param includeStealthModules Whether to include stealth modules. Defaults to true. * @return The reject contains the error message, else contains the sections. */ - async getSections( + getSections( courseId: number, excludeModules: boolean = false, excludeContents: boolean = false, @@ -916,63 +936,83 @@ export class CoreCourseProvider { siteId?: string, includeStealthModules: boolean = true, ): Promise { - - const site = await CoreSites.getSite(siteId); - preSets = preSets || {}; - preSets.cacheKey = this.getSectionsCacheKey(courseId); - preSets.updateFrequency = preSets.updateFrequency || CoreSite.FREQUENCY_RARELY; - - const params: CoreCourseGetContentsParams = { - courseid: courseId, - }; - params.options = [ - { - name: 'excludemodules', - value: excludeModules, - }, - { - name: 'excludecontents', - value: excludeContents, - }, - ]; - - if (this.canRequestStealthModules(site)) { - params.options.push({ - name: 'includestealthmodules', - value: includeStealthModules, - }); - } - - let sections: CoreCourseGetContentsWSSection[]; - try { - sections = await site.read('core_course_get_contents', params, preSets); - } catch { - // Error getting the data, it could fail because we added a new parameter and the call isn't cached. - // Retry without the new parameter and forcing cache. - preSets.omitExpires = true; - params.options.splice(-1, 1); - sections = await site.read('core_course_get_contents', params, preSets); - } - - const siteHomeId = site.getSiteHomeId(); - let showSections = true; - if (courseId == siteHomeId) { - const storedNumSections = site.getStoredConfig('numsections'); - showSections = storedNumSections !== undefined && !!storedNumSections; - } - - if (showSections !== undefined && !showSections && sections.length > 0) { - // Get only the last section (Main menu block section). - sections.pop(); - } - - // Add course to all modules. - return sections.map((section) => ({ - ...section, - modules: section.modules.map((module) => this.addAdditionalModuleData(module, courseId, section.id)), + return firstValueFrom(this.getSectionsObservable(courseId, { + excludeModules, + excludeContents, + includeStealthModules, + preSets, + siteId, })); } + /** + * Get the course sections. + * + * @param courseId The course ID. + * @param options Options. + * @return Observable that returns the sections. + */ + getSectionsObservable( + courseId: number, + options: CoreCourseGetSectionsOptions = {}, + ): Observable { + options.includeStealthModules = options.includeStealthModules ?? true; + + return asyncObservable(async () => { + const site = await CoreSites.getSite(options.siteId); + + const preSets: CoreSiteWSPreSets = { + ...options.preSets, + cacheKey: this.getSectionsCacheKey(courseId), + updateFrequency: CoreSite.FREQUENCY_RARELY, + ...CoreSites.getReadingStrategyPreSets(options.readingStrategy), + }; + + const params: CoreCourseGetContentsParams = { + courseid: courseId, + }; + params.options = [ + { + name: 'excludemodules', + value: !!options.excludeModules, + }, + { + name: 'excludecontents', + value: !!options.excludeContents, + }, + ]; + + if (this.canRequestStealthModules(site)) { + params.options.push({ + name: 'includestealthmodules', + value: !!options.includeStealthModules, + }); + } + + return site.readObservable('core_course_get_contents', params, preSets).pipe( + map(sections => { + const siteHomeId = site.getSiteHomeId(); + let showSections = true; + if (courseId == siteHomeId) { + const storedNumSections = site.getStoredConfig('numsections'); + showSections = storedNumSections !== undefined && !!storedNumSections; + } + + if (showSections !== undefined && !showSections && sections.length > 0) { + // Get only the last section (Main menu block section). + sections.pop(); + } + + // Add course to all modules. + return sections.map((section) => ({ + ...section, + modules: section.modules.map((module) => this.addAdditionalModuleData(module, courseId, section.id)), + })); + }), + ); + }); + } + /** * Get cache key for section WS call. * @@ -1933,3 +1973,13 @@ export type CoreCourseStoreModuleViewedOptions = { timeaccess?: number; siteId?: string; }; + +/** + * Options for getSections. + */ +export type CoreCourseGetSectionsOptions = CoreSitesCommonWSOptions & { + excludeModules?: boolean; + excludeContents?: boolean; + includeStealthModules?: boolean; // Defaults to true. + preSets?: CoreSiteWSPreSets; +}; diff --git a/src/core/features/filter/services/filter-helper.ts b/src/core/features/filter/services/filter-helper.ts index 6467f1b67..ea3fbfce4 100644 --- a/src/core/features/filter/services/filter-helper.ts +++ b/src/core/features/filter/services/filter-helper.ts @@ -15,7 +15,7 @@ import { Injectable } from '@angular/core'; import { CoreNetwork } from '@services/network'; -import { CoreSites } from '@services/sites'; +import { CoreSites, CoreSitesReadingStrategy } from '@services/sites'; import { CoreFilterDelegate } from './filter-delegate'; import { CoreFilter, @@ -31,6 +31,7 @@ import { CoreEvents, CoreEventSiteData } from '@singletons/events'; import { CoreLogger } from '@singletons/logger'; import { CoreSite } from '@classes/site'; import { CoreCourseHelper } from '@features/course/services/course-helper'; +import { firstValueFrom } from '@/core/utils/rxjs'; /** * Helper service to provide filter functionalities. @@ -75,7 +76,11 @@ export class CoreFilterHelperProvider { * @return Promise resolved with the contexts. */ async getBlocksContexts(courseId: number, siteId?: string): Promise { - const blocks = await CoreCourse.getCourseBlocks(courseId, siteId); + // Use stale while revalidate, but always use the first value. If data is updated it will be stored in DB. + const blocks = await firstValueFrom(CoreCourse.getCourseBlocksObservable(courseId, { + readingStrategy: CoreSitesReadingStrategy.STALE_WHILE_REVALIDATE, + siteId, + })); const contexts: CoreFiltersGetAvailableInContextWSParamContext[] = []; @@ -153,7 +158,12 @@ export class CoreFilterHelperProvider { * @return Promise resolved with the contexts. */ async getCourseModulesContexts(courseId: number, siteId?: string): Promise { - const sections = await CoreCourse.getSections(courseId, false, true, undefined, siteId); + // Use stale while revalidate, but always use the first value. If data is updated it will be stored in DB. + const sections = await firstValueFrom(CoreCourse.getSectionsObservable(courseId, { + excludeContents: true, + readingStrategy: CoreSitesReadingStrategy.STALE_WHILE_REVALIDATE, + siteId, + })); const contexts: CoreFiltersGetAvailableInContextWSParamContext[] = []; diff --git a/src/core/features/filter/services/filter.ts b/src/core/features/filter/services/filter.ts index 321748b32..a1735684f 100644 --- a/src/core/features/filter/services/filter.ts +++ b/src/core/features/filter/services/filter.ts @@ -15,8 +15,8 @@ import { Injectable } from '@angular/core'; import { CoreNetwork } from '@services/network'; -import { CoreSites } from '@services/sites'; -import { CoreSite } from '@classes/site'; +import { CoreSites, CoreSitesReadingStrategy } from '@services/sites'; +import { CoreSite, CoreSiteWSPreSets } from '@classes/site'; import { CoreWSExternalWarning } from '@services/ws'; import { CoreTextUtils } from '@services/utils/text'; import { CoreFilterDelegate } from './filter-delegate'; @@ -284,13 +284,15 @@ export class CoreFilterProvider { const data: CoreFiltersGetAvailableInContextWSParams = { contexts: contextsToSend, }; - const preSets = { + const preSets: CoreSiteWSPreSets = { cacheKey: this.getAvailableInContextsCacheKey(contextsToSend), updateFrequency: CoreSite.FREQUENCY_RARELY, splitRequest: { param: 'contexts', maxLength: 300, }, + // Use stale while revalidate, but always use the first value. If data is updated it will be stored in DB. + ...CoreSites.getReadingStrategyPreSets(CoreSitesReadingStrategy.STALE_WHILE_REVALIDATE), }; const result = await site.read( diff --git a/src/core/services/sites.ts b/src/core/services/sites.ts index c16638ac3..7217ef250 100644 --- a/src/core/services/sites.ts +++ b/src/core/services/sites.ts @@ -1796,7 +1796,7 @@ export class CoreSitesProvider { getFromCache: false, emergencyCache: false, }; - case CoreSitesReadingStrategy.UPDATE_IN_BACKGROUND: + case CoreSitesReadingStrategy.STALE_WHILE_REVALIDATE: return { updateInBackground: true, getFromCache: true, @@ -2023,7 +2023,7 @@ export const enum CoreSitesReadingStrategy { PREFER_CACHE, ONLY_NETWORK, PREFER_NETWORK, - UPDATE_IN_BACKGROUND, + STALE_WHILE_REVALIDATE, } /** From 52a4322f0d86c198575c89da11e0b8f96bce10ad Mon Sep 17 00:00:00 2001 From: Dani Palou Date: Thu, 8 Sep 2022 08:34:26 +0200 Subject: [PATCH 13/14] MOBILE-3817 core: Create WSObservable type --- .../services/coursecompletion.ts | 5 ++-- src/core/classes/site.ts | 26 +++++++++---------- src/core/features/course/services/course.ts | 7 +++-- .../courses/services/courses-helper.ts | 10 +++---- src/core/features/courses/services/courses.ts | 20 +++++++------- .../features/courses/services/dashboard.ts | 7 +++-- 6 files changed, 36 insertions(+), 39 deletions(-) diff --git a/src/addons/coursecompletion/services/coursecompletion.ts b/src/addons/coursecompletion/services/coursecompletion.ts index 76feeecd4..488c1d7af 100644 --- a/src/addons/coursecompletion/services/coursecompletion.ts +++ b/src/addons/coursecompletion/services/coursecompletion.ts @@ -17,12 +17,11 @@ import { CoreLogger } from '@singletons/logger'; import { CoreSites, CoreSitesCommonWSOptions } from '@services/sites'; import { CoreUtils } from '@services/utils/utils'; import { CoreCourses } from '@features/courses/services/courses'; -import { CoreSite, CoreSiteWSPreSets } from '@classes/site'; +import { CoreSite, CoreSiteWSPreSets, WSObservable } from '@classes/site'; import { CoreStatusWithWarningsWSResponse, CoreWSExternalWarning } from '@services/ws'; import { makeSingleton } from '@singletons'; import { CoreError } from '@classes/errors/error'; import { asyncObservable, firstValueFrom } from '@/core/utils/rxjs'; -import { Observable } from 'rxjs'; import { map } from 'rxjs/operators'; const ROOT_CACHE_KEY = 'mmaCourseCompletion:'; @@ -119,7 +118,7 @@ export class AddonCourseCompletionProvider { getCompletionObservable( courseId: number, options: AddonCourseCompletionGetCompletionOptions = {}, - ): Observable { + ): WSObservable { return asyncObservable(async () => { const site = await CoreSites.getSite(options.siteId); diff --git a/src/core/classes/site.ts b/src/core/classes/site.ts index dfbeef584..e0dcf4853 100644 --- a/src/core/classes/site.ts +++ b/src/core/classes/site.ts @@ -125,7 +125,7 @@ export class CoreSite { protected lastAutoLogin = 0; protected offlineDisabled = false; // eslint-disable-next-line @typescript-eslint/no-explicit-any - protected ongoingRequests: { [cacheId: string]: Observable } = {}; + protected ongoingRequests: { [cacheId: string]: WSObservable } = {}; protected requestQueue: RequestQueueItem[] = []; protected requestQueueTimeout: number | null = null; protected tokenPluginFileWorks?: boolean; @@ -504,10 +504,10 @@ export class CoreSite { * @param method WS method to use. * @param data Data to send to the WS. * @param preSets Extra options. - * @return Observable. + * @return Observable returning the WS data. */ // eslint-disable-next-line @typescript-eslint/no-explicit-any - readObservable(method: string, data: any, preSets?: CoreSiteWSPreSets): Observable { + readObservable(method: string, data: any, preSets?: CoreSiteWSPreSets): WSObservable { preSets = preSets || {}; preSets.getFromCache = preSets.getFromCache ?? true; preSets.saveToCache = preSets.saveToCache ?? true; @@ -535,10 +535,10 @@ export class CoreSite { * @param method WS method to use. * @param data Data to send to the WS. * @param preSets Extra options. - * @return Observable. + * @return Observable returning the WS data. */ // eslint-disable-next-line @typescript-eslint/no-explicit-any - writeObservable(method: string, data: any, preSets?: CoreSiteWSPreSets): Observable { + writeObservable(method: string, data: any, preSets?: CoreSiteWSPreSets): WSObservable { preSets = preSets || {}; preSets.getFromCache = preSets.getFromCache ?? false; preSets.saveToCache = preSets.saveToCache ?? false; @@ -566,7 +566,7 @@ export class CoreSite { * @param method The WebService method to be called. * @param data Arguments to pass to the method. * @param preSets Extra options. - * @return Observable + * @return Observable returning the WS data. * @description * * Sends a webservice request to the site. This method will automatically add the @@ -576,7 +576,7 @@ export class CoreSite { * data hasn't expired. */ // eslint-disable-next-line @typescript-eslint/no-explicit-any - requestObservable(method: string, data: any, preSets: CoreSiteWSPreSets): Observable { + requestObservable(method: string, data: any, preSets: CoreSiteWSPreSets): WSObservable { if (this.isLoggedOut() && !ALLOWED_LOGGEDOUT_WS.includes(method)) { // Site is logged out, it cannot call WebServices. CoreEvents.trigger(CoreEvents.SESSION_EXPIRED, {}, this.id); @@ -665,14 +665,14 @@ export class CoreSite { * @param data Arguments to pass to the method. * @param preSets Extra options related to the site. * @param wsPreSets Extra options related to the WS call. - * @return Observable. + * @return Observable returning the WS data. */ protected performRequest( method: string, data: unknown, preSets: CoreSiteWSPreSets, wsPreSets: CoreWSPreSets, - ): Observable { + ): WSObservable { const subject = new Subject(); const run = async () => { @@ -1903,9 +1903,9 @@ export class CoreSite { * @param readingStrategy Reading strategy. * @return Observable returning site config. */ - getConfigObservable(name?: undefined, readingStrategy?: CoreSitesReadingStrategy): Observable; - getConfigObservable(name: string, readingStrategy?: CoreSitesReadingStrategy): Observable; - getConfigObservable(name?: string, readingStrategy?: CoreSitesReadingStrategy): Observable { + getConfigObservable(name?: undefined, readingStrategy?: CoreSitesReadingStrategy): WSObservable; + getConfigObservable(name: string, readingStrategy?: CoreSitesReadingStrategy): WSObservable; + getConfigObservable(name?: string, readingStrategy?: CoreSitesReadingStrategy): WSObservable { const preSets: CoreSiteWSPreSets = { cacheKey: this.getConfigCacheKey(), ...CoreSites.getReadingStrategyPreSets(readingStrategy), @@ -2403,7 +2403,7 @@ export function chainRequests>( readingStrategy: CoreSitesReadingStrategy | undefined, callback: (data: T, readingStrategy?: CoreSitesReadingStrategy) => O, ): OperatorFunction> { - return (source: Observable) => new Observable<{ data: T; readingStrategy?: CoreSitesReadingStrategy }>(subscriber => { + return (source: WSObservable) => new Observable<{ data: T; readingStrategy?: CoreSitesReadingStrategy }>(subscriber => { let firstValue = true; let isCompleted = false; diff --git a/src/core/features/course/services/course.ts b/src/core/features/course/services/course.ts index d382d49be..16a680440 100644 --- a/src/core/features/course/services/course.ts +++ b/src/core/features/course/services/course.ts @@ -21,7 +21,7 @@ import { CoreLogger } from '@singletons/logger'; import { CoreSitesCommonWSOptions, CoreSites, CoreSitesReadingStrategy } from '@services/sites'; import { CoreTimeUtils } from '@services/utils/time'; import { CoreUtils } from '@services/utils/utils'; -import { CoreSiteWSPreSets, CoreSite } from '@classes/site'; +import { CoreSiteWSPreSets, CoreSite, WSObservable } from '@classes/site'; import { CoreConstants } from '@/core/constants'; import { makeSingleton, Translate } from '@singletons'; import { CoreStatusWithWarningsWSResponse, CoreWSExternalFile, CoreWSExternalWarning } from '@services/ws'; @@ -54,7 +54,6 @@ import { CoreDatabaseCachingStrategy } from '@classes/database/database-table-pr import { SQLiteDB } from '@classes/sqlitedb'; import { CorePlatform } from '@services/platform'; import { CoreTime } from '@singletons/time'; -import { Observable } from 'rxjs'; import { asyncObservable, firstValueFrom } from '@/core/utils/rxjs'; import { map } from 'rxjs/operators'; @@ -417,7 +416,7 @@ export class CoreCourseProvider { * @return Observable that returns the blocks. * @since 3.7 */ - getCourseBlocksObservable(courseId: number, options: CoreSitesCommonWSOptions = {}): Observable { + getCourseBlocksObservable(courseId: number, options: CoreSitesCommonWSOptions = {}): WSObservable { return asyncObservable(async () => { const site = await CoreSites.getSite(options.siteId); @@ -955,7 +954,7 @@ export class CoreCourseProvider { getSectionsObservable( courseId: number, options: CoreCourseGetSectionsOptions = {}, - ): Observable { + ): WSObservable { options.includeStealthModules = options.includeStealthModules ?? true; return asyncObservable(async () => { diff --git a/src/core/features/courses/services/courses-helper.ts b/src/core/features/courses/services/courses-helper.ts index 4e81eac5c..b2cfe3ca8 100644 --- a/src/core/features/courses/services/courses-helper.ts +++ b/src/core/features/courses/services/courses-helper.ts @@ -26,10 +26,10 @@ import { makeSingleton, Translate } from '@singletons'; import { CoreWSExternalFile } from '@services/ws'; import { AddonCourseCompletion } from '@addons/coursecompletion/services/coursecompletion'; import moment from 'moment-timezone'; -import { Observable, of } from 'rxjs'; +import { of } from 'rxjs'; import { firstValueFrom, zipIncludingComplete } from '@/core/utils/rxjs'; import { catchError, map } from 'rxjs/operators'; -import { chainRequests } from '@classes/site'; +import { chainRequests, WSObservable } from '@classes/site'; /** * Helper to gather some common courses functions. @@ -134,7 +134,7 @@ export class CoreCoursesHelperProvider { courses: CoreEnrolledCourseDataWithExtraInfo[], loadCategoryNames: boolean = false, options: CoreSitesCommonWSOptions = {}, - ): Observable { + ): WSObservable { if (!courses.length) { return of([]); } @@ -245,7 +245,7 @@ export class CoreCoursesHelperProvider { */ getUserCoursesWithOptionsObservable( options: CoreCoursesGetWithOptionsOptions = {}, - ): Observable { + ): WSObservable { return CoreCourses.getUserCoursesObservable(options).pipe( chainRequests(options.readingStrategy, (courses, newReadingStrategy) => { @@ -338,7 +338,7 @@ export class CoreCoursesHelperProvider { protected loadCourseCompletedStatus( course: CoreEnrolledCourseDataWithExtraInfo, options: CoreSitesCommonWSOptions = {}, - ): Observable { + ): WSObservable { if (course.completed !== undefined) { // The WebService already returns the completed status, no need to fetch it. return of(course); diff --git a/src/core/features/courses/services/courses.ts b/src/core/features/courses/services/courses.ts index 6c4ea9e43..753605724 100644 --- a/src/core/features/courses/services/courses.ts +++ b/src/core/features/courses/services/courses.ts @@ -14,14 +14,14 @@ import { Injectable } from '@angular/core'; import { CoreSites, CoreSitesCommonWSOptions, CoreSitesReadingStrategy } from '@services/sites'; -import { CoreSite, CoreSiteWSPreSets } from '@classes/site'; +import { CoreSite, CoreSiteWSPreSets, WSObservable } from '@classes/site'; import { makeSingleton } from '@singletons'; import { CoreStatusWithWarningsWSResponse, CoreWarningsWSResponse, CoreWSExternalFile, CoreWSExternalWarning } from '@services/ws'; import { CoreEvents } from '@singletons/events'; import { CoreWSError } from '@classes/errors/wserror'; import { CoreCourseAnyCourseDataWithExtraInfoAndOptions, CoreCourseWithImageAndColor } from './courses-helper'; import { asyncObservable, firstValueFrom, ignoreErrors, zipIncludingComplete } from '@/core/utils/rxjs'; -import { of, Observable } from 'rxjs'; +import { of } from 'rxjs'; import { map } from 'rxjs/operators'; const ROOT_CACHE_KEY = 'mmCourses:'; @@ -510,7 +510,7 @@ export class CoreCoursesProvider { field: string = '', value: string | number = '', options: CoreSitesCommonWSOptions = {}, - ): Observable { + ): WSObservable { return asyncObservable(async () => { const siteId = options.siteId || CoreSites.getCurrentSiteId(); const originalValue = value; @@ -617,7 +617,7 @@ export class CoreCoursesProvider { customFieldName: string, customFieldValue: string, options: CoreSitesCommonWSOptions, - ): Observable { + ): WSObservable { return asyncObservable(async () => { const site = await CoreSites.getSite(options. siteId); @@ -685,7 +685,7 @@ export class CoreCoursesProvider { getCoursesAdminAndNavOptionsObservable( courseIds: number[], options: CoreSitesCommonWSOptions = {}, - ): Observable<{ + ): WSObservable<{ navOptions: CoreCourseUserAdminOrNavOptionCourseIndexed; admOptions: CoreCourseUserAdminOrNavOptionCourseIndexed; }> { @@ -780,7 +780,7 @@ export class CoreCoursesProvider { getUserAdministrationOptionsObservable( courseIds: number[], options: CoreSitesCommonWSOptions = {}, - ): Observable { + ): WSObservable { if (!courseIds || courseIds.length == 0) { return of({}); } @@ -847,7 +847,7 @@ export class CoreCoursesProvider { getUserNavigationOptionsObservable( courseIds: number[], options: CoreSitesCommonWSOptions = {}, - ): Observable { + ): WSObservable { if (!courseIds || courseIds.length == 0) { return of({}); } @@ -939,10 +939,10 @@ export class CoreCoursesProvider { ): Promise { strategy = strategy ?? (preferCache ? CoreSitesReadingStrategy.PREFER_CACHE : undefined); - return this.getUserCoursesObservable({ + return firstValueFrom(this.getUserCoursesObservable({ readingStrategy: strategy, siteId, - }).toPromise(); + })); } /** @@ -951,7 +951,7 @@ export class CoreCoursesProvider { * @param options Options. * @return Observable that returns the courses. */ - getUserCoursesObservable(options: CoreSitesCommonWSOptions = {}): Observable { + getUserCoursesObservable(options: CoreSitesCommonWSOptions = {}): WSObservable { return asyncObservable(async () => { const site = await CoreSites.getSite(options.siteId); diff --git a/src/core/features/courses/services/dashboard.ts b/src/core/features/courses/services/dashboard.ts index eb812b835..fb6aea9ca 100644 --- a/src/core/features/courses/services/dashboard.ts +++ b/src/core/features/courses/services/dashboard.ts @@ -14,12 +14,11 @@ import { Injectable } from '@angular/core'; import { CoreSites, CoreSitesCommonWSOptions } from '@services/sites'; -import { CoreSite, CoreSiteWSPreSets } from '@classes/site'; +import { CoreSite, CoreSiteWSPreSets, WSObservable } from '@classes/site'; import { CoreCourseBlock } from '@features/course/services/course'; import { CoreStatusWithWarningsWSResponse } from '@services/ws'; import { makeSingleton } from '@singletons'; import { CoreError } from '@classes/errors/error'; -import { Observable } from 'rxjs'; import { map } from 'rxjs/operators'; import { asyncObservable, firstValueFrom } from '@/core/utils/rxjs'; @@ -73,7 +72,7 @@ export class CoreCoursesDashboardProvider { * @return Observable that returns the list of blocks. * @since 3.6 */ - getDashboardBlocksFromWSObservable(options: GetDashboardBlocksOptions = {}): Observable { + getDashboardBlocksFromWSObservable(options: GetDashboardBlocksOptions = {}): WSObservable { return asyncObservable(async () => { const site = await CoreSites.getSite(options.siteId); @@ -142,7 +141,7 @@ export class CoreCoursesDashboardProvider { * @param options Options. * @return observable that returns the list of blocks. */ - getDashboardBlocksObservable(options: GetDashboardBlocksOptions = {}): Observable { + getDashboardBlocksObservable(options: GetDashboardBlocksOptions = {}): WSObservable { return this.getDashboardBlocksFromWSObservable(options).pipe(map(blocks => { let mainBlocks: CoreCourseBlock[] = []; let sideBlocks: CoreCourseBlock[] = []; From 2395edbd052f2c0005f775b9a638c3a6bbce77ae Mon Sep 17 00:00:00 2001 From: Dani Palou Date: Thu, 8 Sep 2022 15:07:03 +0200 Subject: [PATCH 14/14] MOBILE-3817 rxjs: Fix zipIncludingComplete completion When the last observable completed it didn't emit pending values --- src/core/utils/rxjs.ts | 55 ++++++++++++++++++------------- src/core/utils/tests/rxjs.test.ts | 25 ++++++++++++++ 2 files changed, 57 insertions(+), 23 deletions(-) diff --git a/src/core/utils/rxjs.ts b/src/core/utils/rxjs.ts index 463b9b332..b583b0fe5 100644 --- a/src/core/utils/rxjs.ts +++ b/src/core/utils/rxjs.ts @@ -126,7 +126,6 @@ type GetObservablesReturnTypes = { [key in keyof T]: T[key] extends Observabl */ type ZipObservableData = { values: T[]; - hasValueForIndex: boolean[]; completed: boolean; subscription?: Subscription; }; @@ -142,50 +141,62 @@ export function zipIncludingComplete[]>( ...observables: T ): Observable> { return new Observable(subscriber => { - const observablesData: ZipObservableData[] = []; let nextIndex = 0; - let numCompleted = 0; let hasErrored = false; + let hasCompleted = false; + + // Before subscribing, initialize the data for all observables. + const observablesData = observables.map(() => { + values: [], + completed: false, + }); // Treat an emitted event. const treatEmitted = (completed = false) => { - if (hasErrored) { + if (hasErrored || hasCompleted) { return; } - if (numCompleted >= observables.length) { - subscriber.complete(); + if (completed) { + // Check if all observables have completed. + const numCompleted = observablesData.reduce((total, data) => total + (data.completed ? 1 : 0), 0); + if (numCompleted === observablesData.length) { + hasCompleted = true; - return; + // Emit all pending values. + const maxValues = observablesData.reduce((maxValues, data) => Math.max(maxValues, data.values.length), 0); + while (nextIndex < maxValues) { + emitNextValue(); + nextIndex++; + } + + subscriber.complete(); + + return; + } } // Check if any observable still doesn't have data for the index. - const notReady = observablesData.some(data => !data.completed && !data.hasValueForIndex[nextIndex]); + const notReady = observablesData.some(data => !data.completed && !(nextIndex in data.values)); if (notReady) { return; } - // For each observable, get the value for the next index, or last value if not present (completed). - const valueToEmit = observablesData.map(observableData => - observableData.values[nextIndex] ?? observableData.values[observableData.values.length - 1]); - + emitNextValue(); nextIndex++; - subscriber.next(> valueToEmit); if (completed) { // An observable was completed, there might be other values to emit. treatEmitted(true); } }; + const emitNextValue = () => { + // For each observable, get the value for the next index, or last value if not present (completed). + const valueToEmit = observablesData.map(observableData => + observableData.values[nextIndex] ?? observableData.values[observableData.values.length - 1]); - // Before subscribing, initialize the data for all observables. - observables.forEach((observable, obsIndex) => { - observablesData[obsIndex] = { - values: [], - hasValueForIndex: [], - completed: false, - }; - }); + subscriber.next(> valueToEmit); + }; observables.forEach((observable, obsIndex) => { const observableData = observablesData[obsIndex]; @@ -193,7 +204,6 @@ export function zipIncludingComplete[]>( observableData.subscription = observable.subscribe({ next: (value) => { observableData.values.push(value); - observableData.hasValueForIndex.push(true); treatEmitted(); }, error: (error) => { @@ -202,7 +212,6 @@ export function zipIncludingComplete[]>( }, complete: () => { observableData.completed = true; - numCompleted++; treatEmitted(true); }, }); diff --git a/src/core/utils/tests/rxjs.test.ts b/src/core/utils/tests/rxjs.test.ts index 934b9cfb0..e7221b610 100644 --- a/src/core/utils/tests/rxjs.test.ts +++ b/src/core/utils/tests/rxjs.test.ts @@ -234,4 +234,29 @@ describe('RXJS Utils', () => { }); }); + it('zipIncludingComplete emits all pending values when last observable completes', () => { + const scheduler = new TestScheduler((actual, expected) => { + expect(actual).toEqual(expected); + }); + + scheduler.run(({ expectObservable, cold }) => { + expectObservable(zipIncludingComplete( + cold('-a-b-|', { + a: 'A1', + b: 'A2', + c: 'A3', + }), + cold('-a-----|', { + a: 'B1', + }), + )).toBe( + '-a-----(b|)', + { + a: ['A1','B1'], + b: ['A2','B1'], + }, + ); + }); + }); + });