ucfopen / udoit Goto Github PK

$base_url is only set on index.php from $_POST['custom_canvas_api_domain']. Everywhere else, it relies on a value to be predefined in localConfig.php

On a page with multiple images with alt text errors, UFIXIT will work on the first one, but all subsequent images will not have any changes made, even though UFIXIT reports "Fixed"

Rewrite the error messages on lines 42 and 103 in process.php and parseResults.php to be more descriptive and offer contact information to help resolve the problem

Currently the browser will reload old JS or CSS files even when changes have been made on production. Either research ways to include a "version" variable and manually update it on commits, or figure out a script to have the browser reload those files by checking the commit id.

Some notes about the README based on my run through the initial setup of this tool on my local machine.

I think it would be useful to include a list of things you'll need to get started, including:

• A development instance of Canvas (or production if you're brave)
• MySQL installed (with version?)
• Any other dependencies.

Some general questions I think could be answered in the README:

"What are my options for development or testing?" (Heroku, local, etc)
"Can I run this against a test instance of Canvas? How?" (at least provide a link to an external resource)

We should probably move the class and method stuff into the wiki.

"How do I get started with automated testing?"

UDOIT indicates that the following HTML is inaccessible:
<p><strong>Bold text</strong></p>

While I could certainly understand a warning in this instance, there are instances where you may want to indicate that a line of text is important, but not a heading.

For instance, if a course has a lot of videos that they have checked for closed captions, allow the user to ignore video errors/suggestions so that they can focus on the other errors/suggestions for now.

Users can interpret this as telling them to style their paragraphs of text as headings. Instead, it should be something like "Avoid using styles for document structure."

Also, add this sentence to the beginning of the description:
"Bold and Italics are used to emphasize text, whereas headings are used to define the structure of the document."

pNotUsedAsHeader is an error right now, but it should be a suggestion. We are not sure of the content creator's intent with a bolded line of text, so it makes more sense as a suggestion. Plus, it's already listed as a suggestion in "What does UDOIT look for?"

This course exhibits the issue: https://webcourses.ucf.edu/courses/1136300

Use the LTI variable substitution feature to request "Canvas.api.domain", which should negate the need for the api base url variable or the referer test in localConfig. This will also allow UDOIT to be installed in one place and accessed from multiple instances of Canvas.

See this page for more information on how to do variable substitution: https://canvas.instructure.com/courses/913512/pages/variable-substitution?module_item_id=6827542

Course where this occurs: https://webcourses.ucf.edu/courses/1077832

In my work on adding heroku support I noticed the udoit submit doesn't handle error state when submitting to progress.php.

If progress.php errors, the interval just keeps running silently without updating the user.

The "What does UDOIT look for?" descriptions are stored in config/localConfig.template.php, and are correct. The error/suggestion descriptions in the scan results are stored in /core/quail/guidelines/translations/en.txt, and need to be updated to be the same as those in the localConfig.

Hi There,

This tools looks great, we are planning to implement it in the coming weeks/months. When demoing it on one of our servers, I realized it requires PHP 5.4. Is this documented somewhere? If not, I'd suggest adding it to the (otherwise excellent) install instructions. If this is documented somewhere, please forgive me!

Thanks.

~ Cooper

The UFIXIT for this should allow the user to choose a heading for the text: H2, H3, and H4

I would include H5 and H6, but the Canvas editor doesn't support them. Should we include them anyway?

This will allow for analytics like most common error/suggestion, course with the most errors, etc.

This could be an element you can open up that would give you a summary of how many of each type of error exists. Errors with zero occurrences would not show up in the summary.

Since the images aren't displayed in the PDF, maybe the best solution is to remove them from the HTML that gets sent to mPDF.

In multiple places UDOIT as well as UFIXIT saves redundant variables in data structures. Reconfigure the structure of the data passed around to only allow values consistent in all applications to be outward facing and other non-consistent variables to be placed in outward-facing argument holder (an array)

When a color contrast issue is discovered, allow the user to select a color with better contrast based on the colors available in the Rich Content editor. These colors should be filtered down to those that provide enough contrast with the background color.

This should be realized by installing PHPUnit via composer and implementing unit tests for internal UDOIT functions.

The quail library included in UDOIT is a modified version of a very old version of QUAIL.

I've exported the google code version here https://github.com/iturgeon/quail-lib

I'd like to update that library it with the changes UDOIT has added and move it to a composer installed library.

Bonus level 2: QUAIL has a few dependencies of it's own, investigate if those can be plucked out.

https://github.com/DaveChild/Text-Statistics under core/quail/common/services/readability/php-text-statistics/

https://github.com/git-mirror/simpletest is under core/tests/simpletest

The course doesn't look like it has a ton of files, but maybe one of the files is causing an issue.

I'd think it'd be super useful for testing to have a fully constructed Canvas course that touches each feature UDOIT identifies.

If we could export that and add it to the source, contributors would be able to more easily test functionality.

This issue is in relation to the tab widget used to switch between "Scan Course" and "View Old Reports"

One should probably be able to select a tab with the Spacebar key, as well as the Enter, key, since the Enter key causes a user to enter and exit Forms mode in JAWS. Switching modes every time you want to change tabs may be confusing and/or cumbersome for these screen reader users.

Note: If you do this, remember to stop propagation of the Spacebar key event, so it doesn't trigger a page down.

The UDOIT logo is loaded here:

https://github.com/ucfcdl/UDOIT/blob/master/assets/css/main.css#L24

but the URL provided is an absolute path to the UCF servers. The asset is included in the project in /assets/img , and should probably be referenced instead. Will reduce traffic on UCF servers and reduce outside dependancies.

This specific course exhibits this issue: https://webcourses.ucf.edu/courses/1083415/pages/barriers-summary

It's currently "user", which will cause an error for a new installation, considering that the database setup SQL creates a table called "users".

@iturgeon and I discussed the utility of detecting if a master api key is present when running the app.

If now, we want to present the user with the ability to paste in their personal developer key.

This would allow them to quickly test UDOIT's capabilities without the need for a master key.

Currently, it just displays videos however they would be displayed in the page. If the content creator just pasted a link to the video, UDOIT will show a link. If the creator inserted the iframe code for the video into the page, it will show up in an iframe. Make it show up in an iframe always.

When a link is contained within the cell of a table that UFIXIT is trying to change to a header, it fails.

Course: https://webcourses.ucf.edu/courses/1108190/
PDF Example: https://dl.dropboxusercontent.com/u/3429807/SYD3410-15Fall%200W59_2015-08-24_2-56-pm%20%282%29.pdf

Currently, when an error description is clicked within a UDOIT Report prior to being fixed, if UFIXIT is enabled for that error otherwise indefinitely, the first error collapses even if multiple instances of the error exists for that course related document (page, discussion, assignment, etc.). If UFIXIT is enabled for that error, similarly only the first error is collapsed, however, an additional error instance number is present when un-collapsed. See images for reference.

Here's the documentation of the changes to the endpoint: https://canvas.beta.instructure.com/doc/api/file.oauth_endpoints.html#post-login-oauth2-token

Basically, when we exchange the single-use code for the user's api key, Canvas also sends along a refresh token and an expiration time. Once the API key is past this expiration time, it will no longer work, and we'll have to use the same API endpoint we used to get the original key, but passing along the refresh token this time, returning a new key, new refresh token, and new expiration time.

To reproduce:

1. Open course A and start a scan, then open course B and start a scan.
2. Course A finishes first, so you click the "Save report as PDF" button.
3. The PDF will download with the filename being based on course B rather than course A.

The config template file currently in use is missing the $db_reports_table variable in the Database Config section.

/* Database Config */
$db_host = '';
$db_user = '';
$db_password = '';
$db_name = '';
$db_user_table = '';

This causes an undefined variable notice when running reports tracing back to Line 82 of process.php:

$sth = $dbh->prepare("
    INSERT INTO
        $db_reports_table
    SET
        user_id=:userid,
        course_id=:courseid,
        file_path=:filepath,
        date_run=NOW(),
        errors=:errors,
        suggestions=:suggestions
");

The lack of this variable results in a failed database insert that triggers the "Ya done goof'd" error.

The fix is to add the missing variable to the Database config section.

Thanks, and let me know if you need any other information.

~ Cooper

Have you, or do you know of anyone that's deployed this to heroku? PHP apps work fine on heroku, BTW, as long as they adhere (or can be easily modified) to 12 factor principles.

Using file system storage for reports stands out as being problematic - it means the app couldn't easily be put behind a load balancer without using NFS or some other shared file system. Typically you'd use database, s3, or some other detached persistence in a separately scaled service.

Thanks! We're interested in hosting this but would like to keep it consistent with our other heroku-hosted LTI apps.