-
Notifications
You must be signed in to change notification settings - Fork 0
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 Broken Links [#137061817] #53
Conversation
…sting files to work properly Changed xml_parser_spec to check for the proper amount of entries Added entry to xml_parser_spec to check that multiple files in same directory worked as intended
MockZip::MockEntry.new("test.dat"), | ||
] | ||
|
||
result = Senkyoshi.iterate_files(MockZip.new(mock_entries)) | ||
assert_equal(result.size, 3) | ||
assert_equal(result.size, 7)# 4 files + 3 directories = 6 |
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.
Put a space before an end-of-line comment.
MockZip::MockEntry.new("test.dat"), | ||
] | ||
|
||
result = Senkyoshi.iterate_files(MockZip.new(mock_entries)) | ||
assert_equal(result.size, 3) | ||
assert_equal(result.size, 7)# 4 files + 3 directories = 6 |
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.
Put a space before an end-of-line comment.
…ed. ("embedded_links.txt" file added) Added a spec that checks if fix_html gives the right base directory if a file actually exists. ("file1_xid-567_1" is the actual file referenced) Changed xml_parser_spec comment to math properly
CGI.unescapeHTML(file.read) | ||
end | ||
@resources.add([@file6]) | ||
@resources.resources.first.location = "#{Dir.pwd}/spec/fixtures/file1__xid-567_1.txt" |
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.
Line is too long. [93/80]
@file5 = Senkyoshi::SenkyoshiFile.new(entry) | ||
|
||
path = "file1__xid-567_1.txt" | ||
#path = "file1__xid-567_1.txt" |
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.
Missing space after #.
@@ -55,6 +56,9 @@ def self.create_canvas_course(resources, zip_name) | |||
course = CanvasCc::CanvasCC::Models::Course.new | |||
course.course_code = zip_name | |||
resources.each do |resource| | |||
# Skips resources that are files. |
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.
should this say aren't
?
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 it should, it was changed but not pushed, we are getting a new version to review now
entry.extract(name) | ||
name | ||
|
||
begin |
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.
I'm not a fan of open ended rescues like this. Something unrelated could fail and it will just get swallowed up in the rescue. Is there another way you can check for failures?
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.
We are going over this now
What is the purpose of skipping directories? |
This fixes broken links in a few different content types such as assignments and announcements.