Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom

Fix: Empty resource items array was breaking drawer load and added resource e2e tests (Issue/111) #112

Merged
merged 8 commits into from
Feb 9, 2024
Merged

Fix: Empty resource items array was breaking drawer load and added resource e2e tests (Issue/111) #112

merged 8 commits into from
Feb 9, 2024

Conversation

lemmyadams
Copy link
Contributor

@lemmyadams lemmyadams commented Oct 12, 2023 •
edited by cahirodoherty-learningpool
Loading

Addresses #111

Chore:

  • Resources tests added (e2e)

Fixes:

  • Empty resource items array was breaking drawer load

@lemmyadams lemmyadams self-assigned this Oct 12, 2023
@cahirodoherty-learningpool
Copy link
Contributor

Currently blocked by issue 3 of adaptlearning/adapt_framework#3472

@oliverfoster
Copy link
Member

Please sort out data loading. Think more about making these tests generic. i.e. all of the test should run on any course, not just on the vanilla data

This was referenced Jan 15, 2024
Closed
Closed
@cahirodoherty-learningpool
Copy link
Contributor

Please sort out data loading. Think more about making these tests generic. i.e. all of the test should run on any course, not just on the vanilla data

Hi Oliver, I believe I have addressed this in a new PR into this branch: #113

Can you have a look an confirm if those are the changes you think are needed please

@cahirodoherty-learningpool
Issue/111 b (#113)
bf98fc1 
* Loading in config to test values

* Formatting updates

* Dynamically calculated item counts

* Calculating item types declared by current content. Removing layout test as it will be theme-dependent

* Removing position tests as these are theme-dependent

* Fix: Prevent empty resourceItems error on drawer click

* Added tests to run on every page as well as the menu

* Splitting helpers and placing for least repetitive use

* Extend resource items for the page with course-level items

* Correcting array concat rather than object and fixed ordering

* Working tests
@cahirodoherty-learningpool cahirodoherty-learningpool changed the title Chore: Added resource e2e tests (Issue/111) Fix: Empty resource items array was breaking drawer load and added resource e2e tests (Issue/111) Jan 25, 2024
cahirodoherty-learningpool
cahirodoherty-learningpool approved these changes Jan 25, 2024
View reviewed changes
Copy link
Contributor

Choose a reason for hiding this comment

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

👀

@cahirodoherty-learningpool
Copy link
Contributor

@oliverfoster Here is the updated resources tests. I've merged them into the original PR for easier viewing

@cahirodoherty-learningpool
Copy link
Contributor

Hi @oliverfoster Thanks for helping me progress the Core PR to fix drawer on location change. These tests are now passing as expected.

Chalk one up for tests! Caught another (albeit minor) bug 🚀

@oliverfoster
Copy link
Member

No worries @cahirodoherty-learningpool

@oliverfoster oliverfoster merged commit 0406121 into master Feb 9, 2024
@oliverfoster oliverfoster deleted the issue/111 branch February 9, 2024 15:35
github-actions bot pushed a commit that referenced this pull request Feb 9, 2024
@semantic-release-bot
Chore(release): 5.4.3 [skip ci]
804c926 
## [5.4.3](v5.4.2...v5.4.3) (2024-02-09)

### Fix

* Empty resource items array was breaking drawer load and added resource e2e tests (Issue/111) (#112) ([0406121](0406121)), closes [#112](#112)
@github-actions GitHub Actions
Copy link

github-actions bot commented Feb 9, 2024

🎉 This PR is included in version 5.4.3 🎉

The release is available on GitHub release

Your semantic-release bot 📦🚀

@oliverfoster oliverfoster mentioned this pull request Feb 9, 2024
Closed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@eleanor-heath eleanor-heath eleanor-heath approved these changes

@ethan-lp ethan-lp ethan-lp approved these changes

@cahirodoherty-learningpool cahirodoherty-learningpool cahirodoherty-learningpool approved these changes

@oliverfoster oliverfoster oliverfoster approved these changes

Assignees

@lemmyadams lemmyadams

Labels
released
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
5 participants
@lemmyadams @cahirodoherty-learningpool @oliverfoster @ethan-lp @eleanor-heath