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

[UI] adds Hedy dashboard & common error detection #4166

Merged
merged 274 commits into from
Sep 13, 2023
Merged

Conversation

karlijnbok
Copy link
Contributor

@karlijnbok karlijnbok commented Mar 22, 2023

Description

  • Adds a button to the for-teachers/class/{{class_info.id}} page to open the live statistics dashboard.
  • Adds a live statistics dashboards page at /live_stats/class/{{class_info.id}}.

@ghost
Copy link

ghost commented Mar 22, 2023

👇 Click on the image for a new way to code review

Review these changes using an interactive CodeSee Map

Legend

CodeSee Map legend

@karlijnbok karlijnbok changed the title Radboud: New Media Lab - improve Hedy dashboard & misconception detection [UI] Radboud: New Media Lab - improve Hedy dashboard & misconception detection Mar 22, 2023
@@ -49,6 +49,7 @@ <h1>{{ class_info.name }}</h1><span class="cursor-pointer" onclick='hedyApp.rena
<button class="green-btn" id="add-student" onclick=$('#add_students_options').toggle();$(this).toggleClass('green-btn');$(this).toggleClass('blue-btn');>{{_('add_students')}}</button>
<button class="green-btn" id="customize-class-button" data-cy="customize_class_button" onclick="window.location.href = '/for-teachers/customize-class/{{class_info.id}}'">{{_('customize_class')}}</button>
{% endif %}
<button class="green-btn" id="live_stats_button" onclick="window.location.href = '/live_stats/class/{{class_info.id}}'">{{_('class_live')}}</button>
Copy link
Member

Choose a reason for hiding this comment

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

class_live I think is a new babel string? Be sure to run the pybabel extraction command (see docs) so it will be added to the pot files and can be translated.

pre-commit-ci bot and others added 2 commits April 6, 2023 13:23
Start on the mock up version of the live dashboard, me and Ariyan looked through it together and he approves. He couldn't approve the changes himself because he wasn't a member of the hedyorg yet.
HcKide and others added 24 commits April 7, 2023 14:06
A new html doc for the pop ups that inherits from the live page, the macro is adjusted to allow a link for the href (although clicking on the item makes it crash right now) and a render function for the pop up page was also added in statistics.py
Fixed the issue where it threw an error when clicking on a struggle item, had to do with passing jinja vars into a macro. Moved the block popup_content into the right location, otherwise it isn't drawn. 
Created a pop up card with transparent background elements to blend out the rest of the dashboard
Added some backend code in preparation to show last ran program on the popup card. Moreover, the dummy data was slightly updated to reflect the version we have in mind for the mock up better.
We need to know which student has been selected to add the correct data to the student details. Good to note: currently only one student can be selected at a time.
Added a height of 120% otherwise the transparent overlay did not cover all the content which made it look off. Fixed an error that crept in previously where I tried linking a student to the page already, but the functionality for that we will introduce at a later stage. 
Therefore it was error proofed in statistics.py
[RU] Implemented clickable popup windows for the common errors section of the dashboard
% student images added for prototype
added student bubbles and margined them (temp images)
Previously style was used for individual items, it was changed to use only tailwind css classes.
Moreover, the spacing of the list items was off and so this was fixed
@jpelay
Copy link
Member

jpelay commented Aug 7, 2023

I changed the way the misconceptions are gathered. I changed the name to exception types (suggestion by @Felienne) and also made them language independent by using the already tracked exceptions per program. This means that the error_history field is not used anymore and it can be removed what do you think @Felienne.

Also, I haven't realized before that the commor exceptions types per class were stored in the customizations table. This is a bad idea, because they are two conceptually different entities, and also if the teacher deletes the customizations they will also delete this important bit of their class information! So I think this should go to the classes table. I wil ldo that tomorrow.

@Felienne
Copy link
Member

Felienne commented Aug 8, 2023

I changed the way the misconceptions are gathered. I changed the name to exception types (suggestion by @Felienne) and also made them language independent by using the already tracked exceptions per program. This means that the error_history field is not used anymore and it can be removed what do you think @Felienne.

Yes, better to remove that field it was a bit of a weird way of storing!

Also, I haven't realized before that the commor exceptions types per class were stored in the customizations table. This is a bad idea, because they are two conceptually different entities, and also if the teacher deletes the customizations they will also delete this important bit of their class information! So I think this should go to the classes table.

Ah what a good catch @jpelay!! This indeed should be stored elsewhere. Maybe on the student rather than on the class (or on the program) after which we can gather them per student/per class (potentially with some caching in the DB otherwise it'll be too expensive?)

I wil ldo that tomorrow.

<3

@jpelay
Copy link
Member

jpelay commented Aug 8, 2023

Ah what a good catch @jpelay!! This indeed should be stored elsewhere. Maybe on the student rather than on the class (or on the program) after which we can gather them per student/per class (potentially with some caching in the DB otherwise it'll be too expensive?)

Sorry, I don't I don't fully understand this 😅. For clarification you mean storing the groups of exceptions a student have incurred per student right?

Right now they're stored line this:

"class": {
  "common_errors": [
      {
          "label": "invalid_command_exception",
          "active": 1,
          "students": ["student1", "student2"]
      }
   ]
}

In which table do you think we could store this information? These groups are already gathered every time the teacher access the Class live page, they are actually being recomputed every time, they're stored in the database only in case the teacher decides to ignore that specific group of exceptions.

@jpelay
Copy link
Member

jpelay commented Aug 23, 2023

I created a new table where we store the common errors for a class. This table has the following structure:

"class_errors": [
    {
      "id": "5c39c2a936f24db1a4935c52fab77cd7", // the class id
      "errors": [
        {
          "id": 0,
          "label": "invalid_command_exception", // the label of the exception group. Must be messages.pot
          "active": 0, // weather the exception is shown to the teacher
          "students": [
            "student1" // list of students that have incurred in this group of exceptions
          ]
        }
      ]
    }
]

Also, I created a new function for better handling of the class customizations. I tested the page with classes with no customizations and it works now.

Comment on lines +834 to +844
else:
# forcefully overwrite oldest error despite not being resolved and set oldest half of the db to
# inactive to free up space
# Todo: could use a better way to handle this
new_id = 0

for i in range(self.MAX_COMMON_ERRORS // 2):
common_errors['errors'][i]['active'] = 0
self.db.update_class_errors(common_errors)

return new_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 don't know if this is really necessary. This enforces a limit of 10 exception groups per class, so if the students of a class have made exceptions belonging to 10 different groups, it'll delete the 5 previous ones to make room for new. I think this is for not to clutter the UI. What do you think, @Felienne, do we leave this?

@jpelay
Copy link
Member

jpelay commented Aug 23, 2023

With that said, @Felienne this is done!!!

@Felienne Felienne assigned Felienne and unassigned jpelay Sep 12, 2023
@Felienne Felienne dismissed their stale review September 13, 2023 06:55

Everything is now good to go!

@Felienne
Copy link
Member

Hi @karlijnbok!!

It took a while but we are ready to merge this, and it will appear on the website soon! Thanks a lot on behalf of the Hedy team!!

Copy link
Member

@Felienne Felienne left a comment

Choose a reason for hiding this comment

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

Let's go!

@mergify
Copy link
Contributor

mergify bot commented Sep 13, 2023

Thank you for contributing! Your pull request is now going on the merge train (choo choo! Do not click update from main anymore, and be sure to allow changes to be pushed to your fork).

@mergify mergify bot merged commit 7e25197 into main Sep 13, 2023
9 checks passed
@mergify mergify bot deleted the radboud-new-media-lab branch September 13, 2023 10:12
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.

5 participants