-
Notifications
You must be signed in to change notification settings - Fork 84
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.
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
fix(user_report_viewed): Stops errors where course ID is 0. #527
Merged
Merged
Changes from all commits
Commits
Show all changes
9 commits
Select commit
Hold shift + click to select a range
b1651f6
add support for course 0 in forum user report events
garemoko c290570
Fix activity id in tests
garemoko f2d4c41
test fixes
garemoko 707343f
fix "all courses" activity id
garemoko 6a4ee19
Merge branch 'master' into user_report_viewed_fix
garemoko 9efeff1
dont test for course extension when there is no course
garemoko 1fbe6c2
code style fixes
garemoko 82357c6
Complete the fix of related user
garemoko 5237bdb
Merge branch 'master' into user_report_viewed_fix
ryasmi File filter
Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
There are no files selected for viewing
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
21 changes: 21 additions & 0 deletions
21
tests/mod_forum/user_report_viewed/existing_report_viewed_all_courses/data.json
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,21 @@ | ||
{ | ||
"user": [ | ||
{ | ||
"id": 1, | ||
"firstname": "loggedin_user_firstname", | ||
"email": "[email protected]" | ||
}, | ||
{ | ||
"id": 2, | ||
"firstname": "viewed_user_firstname", | ||
"email": "[email protected]" | ||
} | ||
], | ||
"course": [ | ||
{ | ||
"id": 1, | ||
"fullname": "test_name", | ||
"lang": "en" | ||
} | ||
] | ||
} |
10 changes: 10 additions & 0 deletions
10
tests/mod_forum/user_report_viewed/existing_report_viewed_all_courses/event.json
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,10 @@ | ||
{ | ||
"id": 1, | ||
"userid": 1, | ||
"relateduserid": 2, | ||
"courseid": 0, | ||
"timecreated": 1433946701, | ||
"objecttable": null, | ||
"objectid": null, | ||
"eventname": "\\mod_forum\\event\\user_report_viewed" | ||
} |
66 changes: 66 additions & 0 deletions
66
tests/mod_forum/user_report_viewed/existing_report_viewed_all_courses/statements.json
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,66 @@ | ||
[ | ||
{ | ||
"actor": { | ||
"name": "loggedin_user_firstname", | ||
"account": { | ||
"homePage": "http:\/\/www.example.org", | ||
"name": "1" | ||
} | ||
}, | ||
"verb": { | ||
"id": "http:\/\/id.tincanapi.com\/verb\/viewed", | ||
"display": { | ||
"en": "viewed" | ||
} | ||
}, | ||
"object": { | ||
"definition": { | ||
"type": "http:\/\/id.tincanapi.com\/activitytype\/user-profile", | ||
"name": { | ||
"en": "forum posts of viewed_user_firstname" | ||
}, | ||
"extensions": { | ||
"https:\/\/moodle.org\/xapi\/extensions\/user_id": 2 | ||
} | ||
}, | ||
"id": "http:\/\/www.example.org\/mod\/forum\/user.php?id=2" | ||
}, | ||
"timestamp": "2015-06-10T15:31:41+01:00", | ||
"context": { | ||
"platform": "Moodle", | ||
"language": "en", | ||
"extensions": { | ||
"http:\/\/lrs.learninglocker.net\/define\/extensions\/info": { | ||
"http:\/\/moodle.org": "1.0.0", | ||
"https:\/\/github.com\/xAPI-vle\/moodle-logstore_xapi": "0.0.0-development", | ||
"event_name": "\\mod_forum\\event\\user_report_viewed", | ||
"event_function": "\\src\\transformer\\events\\mod_forum\\user_report_viewed" | ||
} | ||
}, | ||
"contextActivities": { | ||
"grouping": [ | ||
{ | ||
"id": "http:\/\/www.example.org", | ||
"definition": { | ||
"type": "http:\/\/id.tincanapi.com\/activitytype\/lms", | ||
"name": { | ||
"en": "test_name" | ||
} | ||
} | ||
} | ||
], | ||
"category": [ | ||
{ | ||
"id": "http:\/\/moodle.org", | ||
"definition": { | ||
"type": "http:\/\/id.tincanapi.com\/activitytype\/source", | ||
"name": { | ||
"en": "Moodle" | ||
} | ||
} | ||
} | ||
] | ||
} | ||
} | ||
} | ||
] |
24 changes: 24 additions & 0 deletions
24
tests/mod_forum/user_report_viewed/existing_report_viewed_all_courses/test.php
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,24 @@ | ||
<?php | ||
// This file is part of Moodle - http://moodle.org/ | ||
// | ||
// Moodle is free software: you can redistribute it and/or modify | ||
// it under the terms of the GNU General Public License as published by | ||
// the Free Software Foundation, either version 3 of the License, or | ||
// (at your option) any later version. | ||
// | ||
// Moodle is distributed in the hope that it will be useful, | ||
// but WITHOUT ANY WARRANTY; without even the implied warranty of | ||
// MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the | ||
// GNU General Public License for more details. | ||
// | ||
// You should have received a copy of the GNU General Public License | ||
// along with Moodle. If not, see <http://www.gnu.org/licenses/>. | ||
|
||
namespace tests\mod_forum\user_report_viewed\existing_report_viewed_all_courses; | ||
defined('MOODLE_INTERNAL') || die(); | ||
|
||
class test extends \tests\xapi_test_case { | ||
protected function get_test_dir() { | ||
return __DIR__; | ||
} | ||
} |
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Since we're using the related user ID below for the activity ID, does that mean we should use the related user here? Same comment for the extension below too.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@garemoko
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Yes, definitely. Well spotted!
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
In which case i probably shouldn't have modified the user part of this file, but instead should have just changed the user that is passed in by user_report_viewed.php
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Sweet. Yeah good idea that probably makes the most sense.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@ryansmith94 this is resolved.