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

Offline evaluation support #1434

Draft
wants to merge 20 commits into
base: master
Choose a base branch
from
Draft

Offline evaluation support #1434

wants to merge 20 commits into from

Conversation

flbulgarelli
Copy link
Member

@flbulgarelli flbulgarelli commented Jul 23, 2020

This draft is just a notebook, based on an old unmerged feature of offline suppport.

It contains comments and references to the actual PRs that merge this behaviour:

See:

@@ -5,7 +5,8 @@ ruby '~> 2.3'

gem 'puma'

gem 'mumuki-domain', github: 'mumuki/mumuki-domain'
gem 'mumuki-domain', github: 'mumuki/mumuki-domain', branch: 'feature-cumparsita'
Copy link
Member Author

Choose a reason for hiding this comment

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

This line only adds offline tests. We should merge them instead

Copy link
Member Author

Choose a reason for hiding this comment

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

Local evaluation

@@ -5,7 +5,8 @@ ruby '~> 2.3'

gem 'puma'

gem 'mumuki-domain', github: 'mumuki/mumuki-domain'
gem 'mumuki-domain', github: 'mumuki/mumuki-domain', branch: 'feature-cumparsita'
gem 'mulangjs', path: '../mulang/ghcjslib/gem'
Copy link
Member Author

Choose a reason for hiding this comment

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

Not necessary in the first iteration

Copy link
Member Author

Choose a reason for hiding this comment

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

Local evaluation

@@ -196,6 +196,8 @@ which are granted to be safe and stable.
* `setUpDeleteFiles`
* `setUpDeleteFile`
* `updateButtonsVisibility`
* `mumuki.registerLocalTestRunner`
* `mumuki.registerLocalExpectationsRunner`
Copy link
Member Author

Choose a reason for hiding this comment

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

OK. Perhaps could be namespaced into mumuki.local

This steps may not always be necessary for local evaluation. They should be documented

Copy link
Member Author

Choose a reason for hiding this comment

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

Local evaluation

@@ -204,9 +206,7 @@ which are granted to be safe and stable.
{
"status": "failed",
"guide_finished_by_solution": false,
"class_for_progress_list_item":"progress-list-item text-center danger active",
"html":"...",
Copy link
Member Author

Choose a reason for hiding this comment

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

Although this is not strictly necessary for a first iteration, it has a lot of sense

Copy link
Member Author

Choose a reason for hiding this comment

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

Pre rendering

Copy link
Member Author

Choose a reason for hiding this comment

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

Done here: #1435

//= require rails-ujs
//= require turbolinks
Copy link
Member Author

Choose a reason for hiding this comment

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

Rollback

data: solution
});
return $.ajax(request);
}
Copy link
Member Author

Choose a reason for hiding this comment

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

Local evaluation + local progress + local sync

Copy link
Member Author

Choose a reason for hiding this comment

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

Done in #1443

result.html = result.html || mumuki.renderCorollaryHtml(status, exercise);
} catch (e) {
console.log(`[Mumuki::Laboratory::Bridge] pre-rendering failed ${e}`);
throw e;
Copy link
Member Author

Choose a reason for hiding this comment

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

Pre rendering

Copy link
Member Author

Choose a reason for hiding this comment

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

Done in #1435

editor.getDoc().setValue(content);
});
}

Copy link
Member Author

Choose a reason for hiding this comment

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

???

@@ -7,7 +7,7 @@ mumuki.load(function () {
url: $(this).data('confirmation-url'),
xhrFields: {withCredentials: true},
success: function(data){
mumuki.updateProgressBarAndShowModal(data);
mumuki.updateCurrentExerciseProgressBarAndShowModal(data);
Copy link
Member Author

Choose a reason for hiding this comment

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

Rollback. Heavily backward incompatible

Laboratory.prototype = {

(() => {
class Laboratory {
Copy link
Member Author

Choose a reason for hiding this comment

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

Done #1443

mumuki.load(() => {
// Set global currentExerciseId
const $muExerciseId = $('#mu-exercise-id')[0];
const $muExerciseResource = $('#mu-exercise-resource')[0];
Copy link
Member Author

Choose a reason for hiding this comment

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

Partially done in #1443

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

1 participant