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

Crowd Assessment: Url for url generation #1672

Draft
wants to merge 1 commit into
base: v25.0.00
Choose a base branch
from

Conversation

yookoala
Copy link
Member

@yookoala yookoala commented Oct 6, 2022

Description

  • Use Url class for url generations.
  • Fix require_once path with DIR.

Motivation and Context

  • Fix potential issues with scripts compatibility with new routing through index.php

How Has This Been Tested?

  • CI environment.

* Use Url class for url generations.
* Fix require_once path with __DIR__.
$form = Form::create('crowdAssessment', $gibbon->session->get('absoluteURL').'/modules/'.$session->get('module')."/crowdAssess_view_discuss_postProcess.php?gibbonPlannerEntryID=$gibbonPlannerEntryID&gibbonPlannerEntryHomeworkID=$gibbonPlannerEntryHomeworkID&address=".$_GET['q']."&gibbonPersonID=$gibbonPersonID&replyTo=$replyTo");
$form = Form::create(
'crowdAssessment',
Url::fromModuleRoute('Crowd Assessment', 'crowdAssess_view_discuss_postProcess')
Copy link
Member

Choose a reason for hiding this comment

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

Hi Koala, I've taken a look at this but I think there may be a type of Url that the Url class cannot properly generate. For forms and process pages, the url needs to go directly to the script itself rather than prepending the index.php module route. In this case, testing your changes in a local system, this form will create an error when submitted because it is trying to go to index.php as a front-end page, not the script itself. I'm curious to hear the best way to handle this, do we add a new method to Url, or add a qualifier to the methods used to build the Url after fromModuleRoute is called. What do you think?

Copy link
Member Author

Choose a reason for hiding this comment

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

I wanted to try (in fact I tried to write tests for Crowd Assessment in the past few weeks) but I don't quite know how to use and test it :-(

Can you give me some directions?

Copy link
Member

Choose a reason for hiding this comment

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

Fair enough, this one is a bit tricky to setup for testing! 😅 Ff you go into the Planner, pick a demo Class that you "teach" and then add a new lesson plan. Make sure the lesson plan date is in the past week or so (not the future), then enable the Homework option with a deadline in the future, then enable Online Submission, and finally enable the Crowd Assessment option. This should create a crowd-assessable lesson. Once created, click to View your lesson plan and you can manually add a submission for a student using the Edit option. Once a student has a submission it should show up in the Crowd Assessment module list. Hope this helps!

Copy link
Member Author

Choose a reason for hiding this comment

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

@SKuipers In the demo data, which class do "I" actually "teach"? Or how do I make my account "teach" a class?

Copy link
Member

Choose a reason for hiding this comment

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

In the demo data you'll have the FL07.1, FL07.2, FL07.3 and FL07.4 classes, they often show up at the top of the classes list under My Classes. To add yourself to a class, you can go into Timetable Admin > Course Enrolment by Person (or by Class). Hope this helps!

@yookoala yookoala marked this pull request as draft November 4, 2022 11:41
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants