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.

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

RTO-35842 Allow multiple ids to show function #34

Merged
6 changes: 5 additions & 1 deletion README.md
Original file line number Diff line number Diff line change
@@ -1,6 +1,6 @@
# MoodleRb

[![Build](https://github.com/jobready/moodle-rb/actions/workflows/build.yml/badge.svg)](https://github.com/jobready/moodle-rb/actions/workflows/build.yml)
[![Build](https://github.com/jobready/moodle-rb/actions/workflows/build.yml/badge.svg)](https://github.com/jobready/moodle-rb/actions/workflows/build.yml)

[![Gem Version](https://badge.fury.io/rb/moodle_rb.svg)](https://badge.fury.io/rb/moodle_rb)

Expand Down Expand Up @@ -49,6 +49,10 @@ Show a course
moodle.courses.show(course_id)
```

```
moodle.courses.show(course_ids) # Example: moodle.courses.show(1234, 234, ...)
```

Delete a course
```
moodle.courses.destroy(course_id)
Expand Down
13 changes: 9 additions & 4 deletions lib/moodle_rb/courses.rb
Original file line number Diff line number Diff line change
Expand Up @@ -69,21 +69,26 @@ def create(params)
response.parsed_response.first
end

def show(id)
def show(*id)
ids = id.map.with_index do |item, idx|
[idx.to_s, item]
end.to_h

response = self.class.post(
'/webservice/rest/server.php',
{
:query => query_hash('core_course_get_courses', token),
:body => {
:options => {
:ids => {
'0' => id
}
id: ids
Copy link
Member

@karlcloudy karlcloudy Nov 14, 2024

Choose a reason for hiding this comment

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

This might be my fault as I probably got it wrong in the suggestion, but I've noticed the parameter key name here should be ids

Copy link
Contributor Author

Choose a reason for hiding this comment

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

done

}
}
}.merge(query_options)
)
check_for_errors(response)

return response.parsed_response if response.parsed_response.count > 1
Copy link
Member

Choose a reason for hiding this comment

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

What does the Moodle API do if you call it with course IDs in the list that do not exist? Hopefully it will skip those and give us just the ones it has. If that's the case, it's possible we may only have 1, or even 0, in the response, even though the caller has passed multiple ids and is expecting an array back, so I think we should return an array (even of 0 or 1 item) if the user has passed more than one id.

Copy link
Member

Choose a reason for hiding this comment

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

I was actually going to suggest a change to this to a ternary statement, but then got sidetracked.

I do think we should check the number of ids passed in as described above.


response.parsed_response.first
end

Expand Down
2 changes: 1 addition & 1 deletion lib/moodle_rb/version.rb
Original file line number Diff line number Diff line change
@@ -1,5 +1,5 @@
module MoodleRb
VERSION = '2.1.5' unless defined?(self::VERSION)
VERSION = '2.2.0' unless defined?(self::VERSION)

def self.version
VERSION
Expand Down
Original file line number Diff line number Diff line change
@@ -0,0 +1,38 @@
http_interactions:
- request:
method: post
uri: http://localhost:8888/moodle28/webservice/rest/server.php?moodlewsrestformat=json&wsfunction=core_course_get_courses&wstoken=60fc9c9415259404795094957e4ab32f
body:
string: options[ids][0]=1&options[ids][1]=2
headers: {}

response:
status:
code: 200
message: OK
headers:
Access-Control-Allow-Origin:
- "*"
X-Powered-By:
- PHP/5.6.2
Expires:
- Thu, 01 Jan 1970 00:00:00 GMT
Server:
- Apache/2.2.29 (Unix) mod_fastcgi/2.4.6 mod_wsgi/3.4 Python/2.7.8 PHP/5.6.2 mod_ssl/2.2.29 OpenSSL/0.9.8zd DAV/2 mod_perl/2.0.8 Perl/v5.20.0
Pragma:
- no-cache
Accept-Ranges:
- none
Content-Type:
- application/json
Date:
- Sun, 12 Apr 2015 00:51:46 GMT
Content-Length:
- "707"
Cache-Control:
- private, must-revalidate, pre-check=0, post-check=0, max-age=0
body:
string: "[{\"id\":1,\"shortname\":\"Moodle 2.8\",\"categoryid\":0,\"categorysortorder\":1,\"fullname\":\"Moodle 2.8\",\"idnumber\":\"\",\"summary\":\"<h2 style=\\\"font-size: large;\\\">Moodle<span style=\\\"font-size: 1.8em; color: red;\\\">4<\\/span>Mac<\\/h2>\\r\\n<p>This package for OS&nbsp;X comes with Moodle&nbsp;2.8.5+ and MAMP 3.0.7.3.&nbsp;We hope you will like it!<\\/p>\",\"summaryformat\":1,\"format\":\"site\",\"showgrades\":1,\"newsitems\":3,\"startdate\":0,\"numsections\":1,\"maxbytes\":0,\"showreports\":0,\"visible\":1,\"groupmode\":0,\"groupmodeforce\":0,\"defaultgroupingid\":0,\"timecreated\":1405375287,\"timemodified\":1426316351,\"enablecompletion\":0,\"completionnotify\":0,\"lang\":\"\",\"forcetheme\":\"\",\"courseformatoptions\":[{\"name\":\"numsections\",\"value\":1}]}, {\"id\":2,\"shortname\":\"Moodle 2.8\",\"categoryid\":0,\"categorysortorder\":1,\"fullname\":\"Moodle 2.8\",\"idnumber\":\"\",\"summary\":\"<h2 style=\\\"font-size: large;\\\">Moodle<span style=\\\"font-size: 1.8em; color: red;\\\">4<\\/span>Mac<\\/h2>\\r\\n<p>This package for OS&nbsp;X comes with Moodle&nbsp;2.8.5+ and MAMP 3.0.7.3.&nbsp;We hope you will like it!<\\/p>\",\"summaryformat\":1,\"format\":\"site\",\"showgrades\":1,\"newsitems\":3,\"startdate\":0,\"numsections\":1,\"maxbytes\":0,\"showreports\":0,\"visible\":1,\"groupmode\":0,\"groupmodeforce\":0,\"defaultgroupingid\":0,\"timecreated\":1405375287,\"timemodified\":1426316351,\"enablecompletion\":0,\"completionnotify\":0,\"lang\":\"\",\"forcetheme\":\"\",\"courseformatoptions\":[{\"name\":\"numsections\",\"value\":1}]}]"
http_version:
recorded_at: Sun, 12 Apr 2015 00:51:46 GMT
recorded_with: VCR 2.9.3
21 changes: 17 additions & 4 deletions spec/lib/moodle_rb/courses_spec.rb
Original file line number Diff line number Diff line change
Expand Up @@ -86,11 +86,24 @@
:match_requests_on => [:path], :record => :once
} do
let(:id) { 1 }
let(:result) { course_moodle_rb.show(id) }

specify do
expect(result).to be_a Hash
expect(result['id']).to eq 1
context 'when passing single id' do
let(:result) { course_moodle_rb.show(id) }

specify do
expect(result).to be_a Hash
expect(result['id']).to eq 1
end
end

context 'when passing array of ids' do
let(:result) { course_moodle_rb.show(1, 2) }

specify do
expect(result).to be_a Array
expect(result[0]['id']).to eq 1
expect(result[1]['id']).to eq 2
end
end

context 'when using invalid token' do
Expand Down
Loading