From 21d0dc6821aa49114149d29ac40d5464640b20bc Mon Sep 17 00:00:00 2001 From: Noel De Martin Date: Thu, 11 Jan 2024 15:24:14 +0100 Subject: [PATCH 1/5] MOBILE-3947 reminders: Fix reminder button a11y roles --- .../reminders/components/date/date.html | 2 +- .../components/set-button/set-button.html | 5 ++-- .../components/set-button/set-button.ts | 29 ++++++++++++++++--- 3 files changed, 29 insertions(+), 7 deletions(-) diff --git a/src/core/features/reminders/components/date/date.html b/src/core/features/reminders/components/date/date.html index 69d18876a..917907430 100644 --- a/src/core/features/reminders/components/date/date.html +++ b/src/core/features/reminders/components/date/date.html @@ -4,4 +4,4 @@ + [label]="label" [initialTimebefore]="timebefore" [time]="time" [title]="title" [url]="url" /> diff --git a/src/core/features/reminders/components/set-button/set-button.html b/src/core/features/reminders/components/set-button/set-button.html index 0d9b2a488..803f90ae0 100644 --- a/src/core/features/reminders/components/set-button/set-button.html +++ b/src/core/features/reminders/components/set-button/set-button.html @@ -1,6 +1,7 @@ + [attr.aria-label]="'core.reminders.setareminderfor' | translate : { title: title, label: labelClean }"> + +{{ reminderMessage }} diff --git a/src/core/features/reminders/components/set-button/set-button.ts b/src/core/features/reminders/components/set-button/set-button.ts index a443e9d35..a775ad6a8 100644 --- a/src/core/features/reminders/components/set-button/set-button.ts +++ b/src/core/features/reminders/components/set-button/set-button.ts @@ -32,18 +32,22 @@ export class CoreRemindersSetButtonComponent implements OnInit { @Input() instanceId?: number; @Input() type?: string; @Input() label = ''; - @Input() timebefore?: number; + @Input() initialTimebefore?: number; @Input() time = -1; @Input() title = ''; @Input() url = ''; labelClean = ''; + timebefore?: number; + reminderMessage?: string; /** * @inheritdoc */ ngOnInit(): void { this.labelClean = this.label.replace(':', ''); + + this.setTimebefore(this.initialTimebefore); } /** @@ -86,6 +90,23 @@ export class CoreRemindersSetButtonComponent implements OnInit { this.saveReminder(reminderTime.timeBefore); } + /** + * Update time before. + */ + setTimebefore(timebefore: number | undefined): void { + this.timebefore = timebefore; + + if (this.timebefore !== undefined) { + const reminderTime = this.time - this.timebefore; + + this.reminderMessage = Translate.instant('core.reminders.reminderset', { + $a: CoreTimeUtils.userDate(reminderTime * 1000), + }); + } else { + this.reminderMessage = undefined; + } + } + /** * Save reminder. * @@ -105,18 +126,18 @@ export class CoreRemindersSetButtonComponent implements OnInit { }); if (timebefore === undefined || timebefore === CoreRemindersService.DISABLED) { - this.timebefore = undefined; + this.setTimebefore(undefined); CoreDomUtils.showToast('core.reminders.reminderunset', true); return; } - this.timebefore = timebefore; + this.setTimebefore(timebefore); const reminder: CoreReminderData = { + timebefore, component: this.component, instanceId: this.instanceId, - timebefore: this.timebefore, type: this.type, title: this.label + ' ' + this.title, url: this.url, From 44606242fd79ffb876d6b89036feba44f1ffe873 Mon Sep 17 00:00:00 2001 From: Noel De Martin Date: Thu, 11 Jan 2024 15:25:49 +0100 Subject: [PATCH 2/5] MOBILE-3947 reminders: Fix tests --- .../tests/behat/behat_app.php | 9 +- .../tests/behat/activity_reminders.feature | 28 ++-- .../tests/behat/course_reminders.feature | 17 +-- src/testing/services/behat-dom.ts | 138 +++++++++++++----- src/testing/services/behat-runtime.ts | 21 ++- 5 files changed, 139 insertions(+), 74 deletions(-) diff --git a/local_moodleappbehat/tests/behat/behat_app.php b/local_moodleappbehat/tests/behat/behat_app.php index aeacba67a..c3a5d65ab 100644 --- a/local_moodleappbehat/tests/behat/behat_app.php +++ b/local_moodleappbehat/tests/behat/behat_app.php @@ -781,13 +781,10 @@ class behat_app extends behat_app_helper { /** * Sets a field to the given text value in the app. * - * Currently this only works for input fields which must be identified using a partial or - * exact match on the placeholder text. - * * @Given /^I set the field "((?:[^"]|\\")+)" to "((?:[^"]|\\")*)" in the app$/ - * @param string $field Text identifying field - * @param string $value Value for field - * @throws DriverException If the field set doesn't work + * @param string $field Text identifying the field. + * @param string $value Value to set. In select fields, this can be either the value or text included in the select option. + * @throws DriverException If the field set doesn't work. */ public function i_set_the_field_in_the_app(string $field, string $value) { $field = addslashes_js($field); diff --git a/src/core/features/reminders/tests/behat/activity_reminders.feature b/src/core/features/reminders/tests/behat/activity_reminders.feature index 00c8224a2..10ec6f0f6 100644 --- a/src/core/features/reminders/tests/behat/activity_reminders.feature +++ b/src/core/features/reminders/tests/behat/activity_reminders.feature @@ -16,52 +16,48 @@ Feature: Set a new reminder on activity | assign | C1 | assign01 | Assignment 01 | ## yesterday ## | ## now +70 minutes ## | | assign | C1 | assign02 | Assignment 02 | ## yesterday ## | ## 1 January 2050 ## | - @ionic7_failure Scenario: Add, delete and update reminder on activity Given I entered the assign activity "Assignment 01" on course "Course 1" as "student1" in the app Then I should not find "Set a reminder for \"Assignment 01\" (Opened)" in the app - And I should find "Set a reminder for \"Assignment 01\" (Due)" in the app - And "Set a reminder for \"Assignment 01\" (Due)" should not be selected in the app + And I should not find "Reminder set for" in the app + But I should find "Set a reminder for \"Assignment 01\" (Due)" in the app # Default set When I press "Set a reminder for \"Assignment 01\" (Due)" in the app - Then I should find "Reminder set for " in the app - And "Set a reminder for \"Assignment 01\" (Due)" should be selected in the app + Then I should find "Reminder set for" in the app # Set from list When I press "Set a reminder for \"Assignment 01\" (Due)" in the app Then I should find "Set a reminder" in the app And "At the time of the event" should be selected in the app - And "1 hour before" should not be selected in the app + But "1 hour before" should not be selected in the app When I press "1 hour before" in the app - Then I should find "Reminder set for " in the app - And "Set a reminder for \"Assignment 01\" (Due)" should be selected in the app + Then I should find "Reminder set for" in the app # Custom set When I press "Set a reminder for \"Assignment 01\" (Due)" in the app Then I should find "Set a reminder" in the app - And "At the time of the event" should not be selected in the app And "1 hour before" should be selected in the app + But "At the time of the event" should not be selected in the app When I press "Custom..." in the app Then I should find "Custom reminder" in the app When I set the following fields to these values in the app: | Value | 4 | | Units | minutes | And I press "Set reminder" in the app - Then I should find "Reminder set for " in the app - And "Set a reminder for \"Assignment 01\" (Due)" should be selected in the app + Then I should find "Reminder set for" in the app # Remove When I press "Set a reminder for \"Assignment 01\" (Due)" in the app Then "4 minutes before" should be selected in the app When I press "Delete reminder" in the app Then I should find "Reminder deleted" in the app - And "Set a reminder for \"Assignment 01\" (Due)" should not be selected in the app + But I should not find "Reminder set for" in the app # Set and check reminder When I press "Set a reminder for \"Assignment 01\" (Due)" in the app - Then I should find "Reminder set for " in the app + Then I should find "Reminder set for" in the app When I press "Set a reminder for \"Assignment 01\" (Due)" in the app And I press "Custom..." in the app Then I should find "Custom reminder" in the app @@ -69,7 +65,7 @@ Feature: Set a new reminder on activity | Value | 69 | | Units | minutes | And I press "Set reminder" in the app - Then I should find "Reminder set for " in the app + Then I should find "Reminder set for" in the app When I wait "50" seconds Then a notification with title "Due: Assignment 01" is present in the app And I close a notification with title "Due: Assignment 01" in the app @@ -82,9 +78,9 @@ Feature: Set a new reminder on activity | Value | 68 | | Units | minutes | And I press "Set reminder" in the app - Then I should find "Reminder set for " in the app + Then I should find "Reminder set for" in the app When I press "Set a reminder for \"Assignment 01\" (Due)" in the app - Then I should find "Reminder set for " in the app + Then I should find "Reminder set for" in the app When I press "Delete reminder" in the app Then I should find "Reminder deleted" in the app When I wait "50" seconds diff --git a/src/core/features/reminders/tests/behat/course_reminders.feature b/src/core/features/reminders/tests/behat/course_reminders.feature index 670d1d8c8..c5dc56e8a 100644 --- a/src/core/features/reminders/tests/behat/course_reminders.feature +++ b/src/core/features/reminders/tests/behat/course_reminders.feature @@ -12,34 +12,33 @@ Feature: Set a new reminder on course | user | course | role | | student1 | C1 | student | - @ionic7_failure Scenario: Add, delete and update reminder on course Given I entered the course "Course 1" as "student1" in the app And I press "Course summary" in the app Then I should not find "Set a reminder for \"Course 1\" (Course start date)" in the app - And I should find "Set a reminder for \"Course 1\" (Course end date)" in the app - And "Set a reminder for \"Course 1\" (Course end date)" should not be selected in the app + And I should not find "Reminder set for" in the app + But I should find "Set a reminder for \"Course 1\" (Course end date)" in the app # Default set When I press "Set a reminder for \"Course 1\" (Course end date)" in the app Then I should find "Reminder set for " in the app - And "Set a reminder for \"Course 1\" (Course end date)" should be selected in the app + And I should find "Reminder set for" in the app # Set from list When I press "Set a reminder for \"Course 1\" (Course end date)" in the app Then I should find "Set a reminder" in the app And "At the time of the event" should be selected in the app - And "12 hours before" should not be selected in the app + But "12 hours before" should not be selected in the app When I press "12 hours before" in the app Then I should find "Reminder set for " in the app - And "Set a reminder for \"Course 1\" (Course end date)" should be selected in the app + And I should find "Reminder set for" in the app # Custom set When I press "Set a reminder for \"Course 1\" (Course end date)" in the app Then I should find "Set a reminder" in the app And "At the time of the event" should not be selected in the app - And "12 hours before" should be selected in the app + But "12 hours before" should be selected in the app When I press "Custom..." in the app Then I should find "Custom reminder" in the app When I set the following fields to these values in the app: @@ -47,11 +46,11 @@ Feature: Set a new reminder on course | Units | hours | And I press "Set reminder" in the app Then I should find "Reminder set for " in the app - And "Set a reminder for \"Course 1\" (Course end date)" should be selected in the app + And I should find "Reminder set for" in the app # Remove When I press "Set a reminder for \"Course 1\" (Course end date)" in the app Then "2 hours before" should be selected in the app When I press "Delete reminder" in the app Then I should find "Reminder deleted" in the app - And "Set a reminder for \"Course 1\" (Course end date)" should not be selected in the app + But I should not find "Reminder set for" in the app diff --git a/src/testing/services/behat-dom.ts b/src/testing/services/behat-dom.ts index 76c812564..8d8fff76a 100644 --- a/src/testing/services/behat-dom.ts +++ b/src/testing/services/behat-dom.ts @@ -276,7 +276,7 @@ export class TestingBehatDomUtilsService { /** * Given a list of elements, get the top ancestors among all of them. * - * This will remote duplicates and drop any elements nested within each other. + * This will remove duplicates and drop any elements nested within each other. * * @param elements Elements list. * @returns Top ancestors. @@ -480,6 +480,34 @@ export class TestingBehatDomUtilsService { return this.findElementsBasedOnText(locator, options)[0]; } + /** + * Wait until an element with the given selector is found. + * + * @param selector Element selector. + * @param timeout Timeout after which an error is thrown. + * @param retryFrequency Frequency for retries when the element is not found. + * @returns Element. + */ + async waitForElement( + selector: string, + timeout: number = 2000, + retryFrequency: number = 100, + ): Promise { + const element = document.querySelector(selector); + + if (!element) { + if (timeout < retryFrequency) { + throw new Error(`Element with '${selector}' selector not found`); + } + + await new Promise(resolve => setTimeout(resolve, retryFrequency)); + + return this.waitForElement(selector, timeout - retryFrequency, retryFrequency); + } + + return element; + } + /** * Function to find elements based on their text or Aria label. * @@ -515,7 +543,7 @@ export class TestingBehatDomUtilsService { protected findElementsBasedOnTextInContainer( locator: TestingBehatElementLocator, topContainer: HTMLElement, - options: TestingBehatFindOptions, + options: TestingBehatFindOptions = {}, ): HTMLElement[] { let container: HTMLElement | null = topContainer; @@ -667,37 +695,26 @@ export class TestingBehatDomUtilsService { } /** - * Set an element value. + * Set an input element value. * - * @param element HTML to set. - * @param value Value to be set. + * @param element Input element. + * @param value Value. */ - async setElementValue(element: HTMLInputElement | HTMLElement, value: string): Promise { + async setInputValue(element: HTMLInputElement | HTMLElement, value: string): Promise { await NgZone.run(async () => { - const promise = new CorePromisedValue(); - // Functions to get/set value depending on field type. - const setValue = (text: string) => { - if (! ('value' in element)) { - element.innerHTML = text; - - return; - } - + const setValue = async (text: string) => { if (element.tagName === 'ION-SELECT') { - value = value.trim(); - const optionValue = Array.from(element.querySelectorAll('ion-select-option')) - .find((option) => option.innerHTML.trim() === value); - - if (optionValue) { - element.value = optionValue.value; - } - } else { + this.setIonSelectInputValue(element, value); + } else if ('value' in element) { element.value = text; + } else { + element.innerHTML = text; } element.dispatchEvent(new Event('ionChange')); }; + const getValue = () => { if ('value' in element) { return element.value; @@ -707,38 +724,79 @@ export class TestingBehatDomUtilsService { }; // Pretend we have cut and pasted the new text. - let event: InputEvent; - if (getValue() !== '') { - event = new InputEvent('input', { + if (element.tagName !== 'ION-SELECT' && getValue() !== '') { + await CoreUtils.nextTick(); + await setValue(''); + + element.dispatchEvent(new InputEvent('input', { bubbles: true, view: window, cancelable: true, inputType: 'deleteByCut', - }); - - await CoreUtils.nextTick(); - setValue(''); - element.dispatchEvent(event); + })); } if (value !== '') { - event = new InputEvent('input', { + await CoreUtils.nextTick(); + await setValue(value); + + element.dispatchEvent(new InputEvent('input', { bubbles: true, view: window, cancelable: true, inputType: 'insertFromPaste', data: value, - }); + })); + } + }); + } - await CoreUtils.nextTick(); - setValue(value); - element.dispatchEvent(event); + /** + * Select an option in an ion-select element. + * + * @param element IonSelect element. + * @param value Value. + */ + protected async setIonSelectInputValue(element: HTMLElement, value: string): Promise { + // Press select. + await TestingBehatDomUtils.pressElement(element); + + // Press option. + type IonSelectInterface = 'alert' | 'action-sheet' | 'popover'; + const selectInterface = element.getAttribute('interface') as IonSelectInterface ?? 'alert'; + const containerSelector = ({ + 'alert': 'ion-alert.select-alert', + 'action-sheet': 'ion-action-sheet.select-action-sheet', + 'popover': 'ion-popover.select-popover', + })[selectInterface]; + const optionSelector = ({ + 'alert': 'button', + 'action-sheet': 'button', + 'popover': 'ion-radio', + })[selectInterface] ?? ''; + const optionsContainer = await TestingBehatDomUtils.waitForElement(containerSelector); + const options = this.findElementsBasedOnTextInContainer( + { text: value, selector: optionSelector }, + optionsContainer, + {}, + ); + + if (options.length === 0) { + throw new Error('Couldn\'t find ion-select option.'); + } + + await TestingBehatDomUtils.pressElement(options[0]); + + // Press options submit. + if (selectInterface === 'alert') { + const submitButton = optionsContainer.querySelector('.alert-button-group button:last-child'); + + if (!submitButton) { + throw new Error('Couldn\'t find ion-select submit button.'); } - promise.resolve(); - - return promise; - }); + await TestingBehatDomUtils.pressElement(submitButton); + } } } diff --git a/src/testing/services/behat-runtime.ts b/src/testing/services/behat-runtime.ts index 9a6e7d5fc..e7813e513 100644 --- a/src/testing/services/behat-runtime.ts +++ b/src/testing/services/behat-runtime.ts @@ -12,7 +12,7 @@ // See the License for the specific language governing permissions and // limitations under the License. -import { TestingBehatDomUtils } from './behat-dom'; +import { TestingBehatDomUtils, TestingBehatDomUtilsService } from './behat-dom'; import { TestingBehatBlocking } from './behat-blocking'; import { CoreCustomURLSchemes, CoreCustomURLSchemesProvider } from '@services/urlschemes'; import { ONBOARDING_DONE } from '@features/login/constants'; @@ -63,6 +63,10 @@ export class TestingBehatRuntimeService { return CoreNavigator.instance; } + get domUtils(): TestingBehatDomUtilsService { + return TestingBehatDomUtils.instance; + } + /** * Init behat functions and set options like skipping onboarding. * @@ -468,11 +472,22 @@ export class TestingBehatRuntimeService { ?? options.find(option => option.text === value)?.value ?? options.find(option => option.text.includes(value))?.value ?? value; + } else if (input.tagName === 'ION-SELECT') { + const options = Array.from(input.querySelectorAll('ion-select-option')); + + value = options.find(option => option.value?.toString() === value)?.textContent?.trim() + ?? options.find(option => option.textContent?.trim() === value)?.textContent?.trim() + ?? options.find(option => option.textContent?.includes(value))?.textContent?.trim() + ?? value; } - await TestingBehatDomUtils.setElementValue(input, value); + try { + await TestingBehatDomUtils.setInputValue(input, value); - return 'OK'; + return 'OK'; + } catch (error) { + return `ERROR: ${error.message ?? 'Unknown error'}`; + } } /** From 2200669b0fa2a5d54b20d9d1548ff1b1338f9f3d Mon Sep 17 00:00:00 2001 From: Noel De Martin Date: Thu, 11 Jan 2024 15:26:49 +0100 Subject: [PATCH 3/5] MOBILE-3947 messages: Fix tests --- .../messages/tests/behat/basic_usage.feature | 2 -- src/app/app-routing.module.ts | 14 ++++++++++++++ .../mainmenu/mainmenu-tab-routing.module.ts | 4 +++- 3 files changed, 17 insertions(+), 3 deletions(-) diff --git a/src/addons/messages/tests/behat/basic_usage.feature b/src/addons/messages/tests/behat/basic_usage.feature index eb3621b0f..db30ff172 100755 --- a/src/addons/messages/tests/behat/basic_usage.feature +++ b/src/addons/messages/tests/behat/basic_usage.feature @@ -280,7 +280,6 @@ Feature: Test basic usage of messages in app Then I should find "Teacher teacher" in the app And I should find "Student1 student1" in the app - @ionic7_failure Scenario: User blocking feature Given I entered the course "Course 1" as "student2" in the app When I press "Participants" in the app @@ -318,7 +317,6 @@ Feature: Test basic usage of messages in app Then I should find "test message" in the app But I should not find "You are unable to message this user" in the app - @ionic7_failure Scenario: Mute Unmute conversations Given I entered the course "Course 1" as "student1" in the app When I press "Participants" in the app diff --git a/src/app/app-routing.module.ts b/src/app/app-routing.module.ts index 869d3af27..b75eaba23 100644 --- a/src/app/app-routing.module.ts +++ b/src/app/app-routing.module.ts @@ -169,6 +169,20 @@ export function conditionalRoutes(routes: Routes, condition: () => boolean): Rou }); } +/** + * Check whether a route does not have any content. + * + * @param route Route. + * @returns Whether the route doesn't have any content. + */ +export function isEmptyRoute(route: Route): boolean { + return !('component' in route) + && !('loadComponent' in route) + && !('children' in route) + && !('loadChildren' in route) + && !('redirectTo' in route); +} + /** * Resolve module routes. * diff --git a/src/core/features/mainmenu/mainmenu-tab-routing.module.ts b/src/core/features/mainmenu/mainmenu-tab-routing.module.ts index 6218138f1..952582c39 100644 --- a/src/core/features/mainmenu/mainmenu-tab-routing.module.ts +++ b/src/core/features/mainmenu/mainmenu-tab-routing.module.ts @@ -15,7 +15,7 @@ import { InjectionToken, Injector, ModuleWithProviders, NgModule } from '@angular/core'; import { Route, Routes } from '@angular/router'; -import { ModuleRoutesConfig, resolveModuleRoutes } from '@/app/app-routing.module'; +import { ModuleRoutesConfig, isEmptyRoute, resolveModuleRoutes } from '@/app/app-routing.module'; const MAIN_MENU_TAB_ROUTES = new InjectionToken('MAIN_MENU_TAB_ROUTES'); const modulesPaths: Record> = {}; @@ -71,6 +71,8 @@ export function buildTabMainRoutes(injector: Injector, mainRoute: Route): Routes if (isRootRoute && !('redirectTo' in mainRoute)) { mainRoute.children = mainRoute.children || []; mainRoute.children = mainRoute.children.concat(routes.children); + } else if (isEmptyRoute(mainRoute)) { + return []; } return isRootRoute From 3621bc46a5b743ca9f29ef4760a8fd41c27e0728 Mon Sep 17 00:00:00 2001 From: Noel De Martin Date: Thu, 11 Jan 2024 15:33:14 +0100 Subject: [PATCH 4/5] MOBILE-3947 glossary: Fix tests There were some bugs in Angular v10 that countered this bug in our code, that's why it wasn't manifested until now. It seems to be related with the changes in createUrlTreeFromSegmentGroup. See https://github.com/angular/angular/commits/16.0.x/packages/router/src/create_url_tree.ts?since=2020-06-25&until=2024-01-11 --- src/addons/calendar/pages/event/event.ts | 6 ++++-- .../pages/competency/competency.page.ts | 8 +++++--- .../pages/submission-review/submission-review.ts | 6 ++++-- src/addons/mod/feedback/pages/attempt/attempt.ts | 8 +++++--- .../mod/forum/pages/discussion/discussion.ts | 6 ++++-- .../forum/pages/new-discussion/new-discussion.ts | 6 ++++-- src/addons/mod/glossary/pages/entry/entry.ts | 6 ++++-- .../mod/glossary/tests/behat/navigation.feature | 1 - .../pages/notification/notification.ts | 8 +++++--- .../items-management/list-items-manager.ts | 6 ++++-- .../items-management/routed-items-manager.ts | 16 +++++++--------- .../swipe-navigation-items-manager.ts | 6 ++++-- .../features/grades/pages/course/course.page.ts | 6 ++++-- src/core/features/user/pages/profile/profile.ts | 6 ++++-- 14 files changed, 58 insertions(+), 37 deletions(-) diff --git a/src/addons/calendar/pages/event/event.ts b/src/addons/calendar/pages/event/event.ts index 3aedbbeb8..27e64a067 100644 --- a/src/addons/calendar/pages/event/event.ts +++ b/src/addons/calendar/pages/event/event.ts @@ -639,8 +639,10 @@ class AddonCalendarEventsSwipeItemsManager extends CoreSwipeNavigationItemsManag /** * @inheritdoc */ - protected getSelectedItemPathFromRoute(route: ActivatedRouteSnapshot): string | null { - return route.params.id; + protected getSelectedItemPathFromRoute(route: ActivatedRouteSnapshot | ActivatedRoute): string | null { + const snapshot = route instanceof ActivatedRouteSnapshot ? route : route.snapshot; + + return snapshot.params.id; } } diff --git a/src/addons/competency/pages/competency/competency.page.ts b/src/addons/competency/pages/competency/competency.page.ts index 9431a03e9..8273d4428 100644 --- a/src/addons/competency/pages/competency/competency.page.ts +++ b/src/addons/competency/pages/competency/competency.page.ts @@ -36,7 +36,7 @@ import { ADDON_COMPETENCY_SUMMARY_PAGE } from '@addons/competency/competency.mod import { CoreSwipeNavigationItemsManager } from '@classes/items-management/swipe-navigation-items-manager'; import { CoreRoutedItemsManagerSourcesTracker } from '@classes/items-management/routed-items-manager-sources-tracker'; import { AddonCompetencyPlanCompetenciesSource } from '@addons/competency/classes/competency-plan-competencies-source'; -import { ActivatedRouteSnapshot } from '@angular/router'; +import { ActivatedRoute, ActivatedRouteSnapshot } from '@angular/router'; import { AddonCompetencyCourseCompetenciesSource } from '@addons/competency/classes/competency-course-competencies-source'; import { CoreTime } from '@singletons/time'; import { CoreAnalytics, CoreAnalyticsEventType } from '@services/analytics'; @@ -350,8 +350,10 @@ class AddonCompetencyCompetenciesSwipeManager /** * @inheritdoc */ - protected getSelectedItemPathFromRoute(route: ActivatedRouteSnapshot): string | null { - return route.params.competencyId; + protected getSelectedItemPathFromRoute(route: ActivatedRouteSnapshot | ActivatedRoute): string | null { + const snapshot = route instanceof ActivatedRouteSnapshot ? route : route.snapshot; + + return snapshot.params.competencyId; } } diff --git a/src/addons/mod/assign/pages/submission-review/submission-review.ts b/src/addons/mod/assign/pages/submission-review/submission-review.ts index 699ff3013..0c65440a7 100644 --- a/src/addons/mod/assign/pages/submission-review/submission-review.ts +++ b/src/addons/mod/assign/pages/submission-review/submission-review.ts @@ -245,8 +245,10 @@ class AddonModAssignSubmissionSwipeItemsManager extends CoreSwipeNavigationItems /** * @inheritdoc */ - protected getSelectedItemPathFromRoute(route: ActivatedRouteSnapshot): string | null { - return route.params.submitId; + protected getSelectedItemPathFromRoute(route: ActivatedRouteSnapshot | ActivatedRoute): string | null { + const snapshot = route instanceof ActivatedRouteSnapshot ? route : route.snapshot; + + return snapshot.params.submitId; } } diff --git a/src/addons/mod/feedback/pages/attempt/attempt.ts b/src/addons/mod/feedback/pages/attempt/attempt.ts index afb1241e4..58c612a2d 100644 --- a/src/addons/mod/feedback/pages/attempt/attempt.ts +++ b/src/addons/mod/feedback/pages/attempt/attempt.ts @@ -13,7 +13,7 @@ // limitations under the License. import { Component, OnDestroy, OnInit } from '@angular/core'; -import { ActivatedRouteSnapshot } from '@angular/router'; +import { ActivatedRoute, ActivatedRouteSnapshot } from '@angular/router'; import { CoreRoutedItemsManagerSourcesTracker } from '@classes/items-management/routed-items-manager-sources-tracker'; import { CoreSwipeNavigationItemsManager } from '@classes/items-management/swipe-navigation-items-manager'; import { CoreNavigator } from '@services/navigator'; @@ -187,8 +187,10 @@ class AddonModFeedbackAttemptsSwipeManager extends CoreSwipeNavigationItemsManag /** * @inheritdoc */ - protected getSelectedItemPathFromRoute(route: ActivatedRouteSnapshot): string | null { - return route.params.attemptId; + protected getSelectedItemPathFromRoute(route: ActivatedRouteSnapshot | ActivatedRoute): string | null { + const snapshot = route instanceof ActivatedRouteSnapshot ? route : route.snapshot; + + return snapshot.params.attemptId; } } diff --git a/src/addons/mod/forum/pages/discussion/discussion.ts b/src/addons/mod/forum/pages/discussion/discussion.ts index 0779c78fa..bf0f77308 100644 --- a/src/addons/mod/forum/pages/discussion/discussion.ts +++ b/src/addons/mod/forum/pages/discussion/discussion.ts @@ -893,8 +893,10 @@ class AddonModForumDiscussionDiscussionsSwipeManager extends AddonModForumDiscus /** * @inheritdoc */ - protected getSelectedItemPathFromRoute(route: ActivatedRouteSnapshot): string | null { - return this.getSource().DISCUSSIONS_PATH_PREFIX + route.params.discussionId; + protected getSelectedItemPathFromRoute(route: ActivatedRouteSnapshot | ActivatedRoute): string | null { + const snapshot = route instanceof ActivatedRouteSnapshot ? route : route.snapshot; + + return this.getSource().DISCUSSIONS_PATH_PREFIX + snapshot.params.discussionId; } } diff --git a/src/addons/mod/forum/pages/new-discussion/new-discussion.ts b/src/addons/mod/forum/pages/new-discussion/new-discussion.ts index 8dc2a1563..052397773 100644 --- a/src/addons/mod/forum/pages/new-discussion/new-discussion.ts +++ b/src/addons/mod/forum/pages/new-discussion/new-discussion.ts @@ -699,8 +699,10 @@ class AddonModForumNewDiscussionDiscussionsSwipeManager extends AddonModForumDis /** * @inheritdoc */ - protected getSelectedItemPathFromRoute(route: ActivatedRouteSnapshot): string | null { - return `${this.getSource().DISCUSSIONS_PATH_PREFIX}new/${route.params.timeCreated}`; + protected getSelectedItemPathFromRoute(route: ActivatedRouteSnapshot | ActivatedRoute): string | null { + const snapshot = route instanceof ActivatedRouteSnapshot ? route : route.snapshot; + + return `${this.getSource().DISCUSSIONS_PATH_PREFIX}new/${snapshot.params.timeCreated}`; } } diff --git a/src/addons/mod/glossary/pages/entry/entry.ts b/src/addons/mod/glossary/pages/entry/entry.ts index cc15c5dcb..3160c1818 100644 --- a/src/addons/mod/glossary/pages/entry/entry.ts +++ b/src/addons/mod/glossary/pages/entry/entry.ts @@ -367,8 +367,10 @@ class AddonModGlossaryEntryEntriesSwipeManager /** * @inheritdoc */ - protected getSelectedItemPathFromRoute(route: ActivatedRouteSnapshot): string | null { - return `${this.getSource().GLOSSARY_PATH_PREFIX}entry/${route.params.entrySlug}`; + protected getSelectedItemPathFromRoute(route: ActivatedRouteSnapshot | ActivatedRoute): string | null { + const snapshot = route instanceof ActivatedRouteSnapshot ? route : route.snapshot; + + return `${this.getSource().GLOSSARY_PATH_PREFIX}entry/${snapshot.params.entrySlug}`; } } diff --git a/src/addons/mod/glossary/tests/behat/navigation.feature b/src/addons/mod/glossary/tests/behat/navigation.feature index fb6ac072b..855640abb 100644 --- a/src/addons/mod/glossary/tests/behat/navigation.feature +++ b/src/addons/mod/glossary/tests/behat/navigation.feature @@ -211,7 +211,6 @@ Feature: Test glossary navigation And I should find "Cashew" in the app And I should find "Acerola" in the app - @ci_jenkins_skip @ionic7_failure Scenario: Tablet navigation on glossary Given I entered the course "Course 1" as "student1" in the app And I change viewport size to "1200x640" in the app diff --git a/src/addons/notifications/pages/notification/notification.ts b/src/addons/notifications/pages/notification/notification.ts index a8f6258df..79e674175 100644 --- a/src/addons/notifications/pages/notification/notification.ts +++ b/src/addons/notifications/pages/notification/notification.ts @@ -20,7 +20,7 @@ import { AddonNotificationsHelper, } from '@addons/notifications/services/notifications-helper'; import { Component, OnDestroy, OnInit } from '@angular/core'; -import { ActivatedRouteSnapshot } from '@angular/router'; +import { ActivatedRoute, ActivatedRouteSnapshot } from '@angular/router'; import { CoreRoutedItemsManagerSourcesTracker } from '@classes/items-management/routed-items-manager-sources-tracker'; import { CoreSwipeNavigationItemsManager } from '@classes/items-management/swipe-navigation-items-manager'; import { CoreContentLinksAction, CoreContentLinksDelegate } from '@features/contentlinks/services/contentlinks-delegate'; @@ -211,8 +211,10 @@ class AddonNotificationSwipeItemsManager extends CoreSwipeNavigationItemsManager /** * @inheritdoc */ - protected getSelectedItemPathFromRoute(route: ActivatedRouteSnapshot): string | null { - return route.params.id; + protected getSelectedItemPathFromRoute(route: ActivatedRouteSnapshot | ActivatedRoute): string | null { + const snapshot = route instanceof ActivatedRouteSnapshot ? route : route.snapshot; + + return snapshot.params.id; } } diff --git a/src/core/classes/items-management/list-items-manager.ts b/src/core/classes/items-management/list-items-manager.ts index 3ec31d484..035d0b845 100644 --- a/src/core/classes/items-management/list-items-manager.ts +++ b/src/core/classes/items-management/list-items-manager.ts @@ -239,13 +239,15 @@ export class CoreListItemsManager< /** * @inheritdoc */ - protected getSelectedItemPathFromRoute(route: ActivatedRouteSnapshot): string | null { + protected getSelectedItemPathFromRoute(route: ActivatedRouteSnapshot | ActivatedRoute): string | null { const segments: UrlSegment[] = []; while (route.firstChild) { route = route.firstChild; - segments.push(...route.url); + const snapshot = route instanceof ActivatedRouteSnapshot ? route : route.snapshot; + + segments.push(...snapshot.url); } return segments.map(segment => segment.path).join('/').replace(/\/+/, '/').trim() || null; diff --git a/src/core/classes/items-management/routed-items-manager.ts b/src/core/classes/items-management/routed-items-manager.ts index a2e8bf514..c873b81f9 100644 --- a/src/core/classes/items-management/routed-items-manager.ts +++ b/src/core/classes/items-management/routed-items-manager.ts @@ -55,7 +55,7 @@ export abstract class CoreRoutedItemsManager< * @param route Page route. * @returns Path of the selected item in the given route. */ - protected abstract getSelectedItemPathFromRoute(route: ActivatedRouteSnapshot): string | null; + protected abstract getSelectedItemPathFromRoute(route: ActivatedRouteSnapshot | ActivatedRoute): string | null; /** * Get the path of the selected item. @@ -63,7 +63,7 @@ export abstract class CoreRoutedItemsManager< * @param route Page route, if any. * @returns Path of the selected item. */ - protected getSelectedItemPath(route?: ActivatedRouteSnapshot | null): string | null { + protected getSelectedItemPath(route?: ActivatedRouteSnapshot | ActivatedRoute | null): string | null { if (!route) { return null; } @@ -76,14 +76,12 @@ export abstract class CoreRoutedItemsManager< * * @param route Current route. */ - protected updateSelectedItem(route: ActivatedRouteSnapshot | null = null): void { - route = route ?? this.getCurrentPageRoute()?.snapshot ?? null; + protected updateSelectedItem(route: ActivatedRouteSnapshot | ActivatedRoute | null = null): void { + route = route ?? this.getCurrentPageRoute() ?? null; const selectedItemPath = this.getSelectedItemPath(route); + const selectedItem = selectedItemPath ? (this.itemsMap?.[selectedItemPath] ?? null) : null; - const selectedItem = selectedItemPath - ? this.itemsMap?.[selectedItemPath] ?? null - : null; this.setSelectedItem(selectedItem); } @@ -106,7 +104,7 @@ export abstract class CoreRoutedItemsManager< // If this item is already selected, do nothing. const itemPath = this.getSource().getItemPath(item); - const selectedItemPath = this.getSelectedItemPath(route.snapshot); + const selectedItemPath = this.getSelectedItemPath(route); if (selectedItemPath === itemPath) { return; @@ -135,7 +133,7 @@ export abstract class CoreRoutedItemsManager< } // If the current page is already the index, do nothing. - const selectedItemPath = this.getSelectedItemPath(route.snapshot); + const selectedItemPath = this.getSelectedItemPath(route); if (selectedItemPath === null) { return; diff --git a/src/core/classes/items-management/swipe-navigation-items-manager.ts b/src/core/classes/items-management/swipe-navigation-items-manager.ts index f0be57a0c..71e9cb201 100644 --- a/src/core/classes/items-management/swipe-navigation-items-manager.ts +++ b/src/core/classes/items-management/swipe-navigation-items-manager.ts @@ -81,11 +81,13 @@ export class CoreSwipeNavigationItemsManager< /** * @inheritdoc */ - protected getSelectedItemPathFromRoute(route: ActivatedRouteSnapshot): string | null { + protected getSelectedItemPathFromRoute(route: ActivatedRouteSnapshot | ActivatedRoute): string | null { const segments: UrlSegment[] = []; while (route) { - segments.push(...route.url); + const snapshot = route instanceof ActivatedRouteSnapshot ? route : route.snapshot; + + segments.push(...snapshot.url); if (!route.firstChild) { break; diff --git a/src/core/features/grades/pages/course/course.page.ts b/src/core/features/grades/pages/course/course.page.ts index 40d8d4ee4..a715394c3 100644 --- a/src/core/features/grades/pages/course/course.page.ts +++ b/src/core/features/grades/pages/course/course.page.ts @@ -330,8 +330,10 @@ class CoreGradesCourseParticipantsSwipeManager extends CoreSwipeNavigationItemsM /** * @inheritdoc */ - protected getSelectedItemPathFromRoute(route: ActivatedRouteSnapshot): string | null { - return route.params.userId; + protected getSelectedItemPathFromRoute(route: ActivatedRouteSnapshot | ActivatedRoute): string | null { + const snapshot = route instanceof ActivatedRouteSnapshot ? route : route.snapshot; + + return snapshot.params.userId; } } diff --git a/src/core/features/user/pages/profile/profile.ts b/src/core/features/user/pages/profile/profile.ts index e056c0acd..64d97ed6a 100644 --- a/src/core/features/user/pages/profile/profile.ts +++ b/src/core/features/user/pages/profile/profile.ts @@ -253,8 +253,10 @@ class CoreUserSwipeItemsManager extends CoreSwipeNavigationItemsManager { /** * @inheritdoc */ - protected getSelectedItemPathFromRoute(route: ActivatedRouteSnapshot): string | null { - return route.params.userId; + protected getSelectedItemPathFromRoute(route: ActivatedRouteSnapshot | ActivatedRoute): string | null { + const snapshot = route instanceof ActivatedRouteSnapshot ? route : route.snapshot; + + return snapshot.params.userId; } } From cf098ea45cb47724c48e4f88300600a292a0fec6 Mon Sep 17 00:00:00 2001 From: Noel De Martin Date: Thu, 11 Jan 2024 15:59:35 +0100 Subject: [PATCH 5/5] MOBILE-3947 behat: Remove ionic7 failure skips --- .github/workflows/acceptance.yml | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/.github/workflows/acceptance.yml b/.github/workflows/acceptance.yml index 2cbc29c28..96e5d2998 100644 --- a/.github/workflows/acceptance.yml +++ b/.github/workflows/acceptance.yml @@ -6,7 +6,7 @@ on: behat_tags: description: 'Behat tags to execute' required: true - default: '~@performance&&~@ionic7_failure' + default: '~@performance' moodle_branch: description: 'Moodle branch' required: true @@ -27,7 +27,7 @@ jobs: MOODLE_DOCKER_PHP_VERSION: '8.1' MOODLE_BRANCH: ${{ github.event.inputs.moodle_branch || 'main' }} MOODLE_REPOSITORY: ${{ github.event.inputs.moodle_repository || 'https://github.com/moodle/moodle' }} - BEHAT_TAGS: ${{ github.event.inputs.behat_tags || '~@performance&&~@ionic7_failure' }} + BEHAT_TAGS: ${{ github.event.inputs.behat_tags || '~@performance' }} steps: - uses: actions/checkout@v2