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

E2229 Track the time students look at other submissions #2377

Closed
wants to merge 21 commits into from

Conversation

Jmanderson1997
Copy link
Contributor

Please set the title of your pull request in the following format: [Project id/Independent study/Issue fix/...]. project name, eg., E904. Integrating courses and assignments on Expertiza with Moodle

We suggest you including the below information in your pull request:

  • A description of the changes proposed in the pull request.
  • @mentions of the person or team responsible for reviewing proposed changes.

About Expertiza Bot

  • If you have any questions about comments given by the expertiza-bot (example), you could create a comment in your pull request with the format /dispute [UUID1] [UUID2] (example) then the professor and TAs will be notified.
  • Here is a video about how to communicate to expertiza-bot.

@coveralls
Copy link

Coverage Status

Coverage decreased (-34.1%) to 17.213% when pulling 96f8935 on Jmanderson1997:beta into 222423a on expertiza:beta.

@coveralls
Copy link

coveralls commented Apr 17, 2022

Coverage Status

Coverage decreased (-33.8%) to 17.382% when pulling 47f22da on Jmanderson1997:beta into 44cbab0 on expertiza:beta.

@coveralls
Copy link

Coverage Status

Coverage decreased (-34.1%) to 17.213% when pulling 96f8935 on Jmanderson1997:beta into 222423a on expertiza:beta.

@expertiza-travisci-bot
Copy link

Hey @Jmanderson1997,
Your changes look good to me! 🎉


</table>

</div>

<% def get_link_round_review_time(map_id, round, link) %>
Copy link
Collaborator

Choose a reason for hiding this comment

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

functions should not be defined in views

<% end %>

<div id="submissionviewingevent">
<% reviewee_list = Array.new %>
Copy link
Collaborator

Choose a reason for hiding this comment

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

Suggested change
<% reviewee_list = Array.new %>
<% reviewee_list = [] %>

use this syntax when you don't initialize the array to anything specific


<div id="submissionviewingevent">
<% reviewee_list = Array.new %>
<% total_time_list = Array.new %>
Copy link
Collaborator

Choose a reason for hiding this comment

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

Suggested change
<% total_time_list = Array.new %>
<% total_time_list = [] %>

<div id="submissionviewingevent">
<% reviewee_list = Array.new %>
<% total_time_list = Array.new %>
<% timeHash = Hash.new %>
Copy link
Collaborator

Choose a reason for hiding this comment

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

Suggested change
<% timeHash = Hash.new %>
<% time_hash = {} %>

<% else %>
<% link_list.push(submissionviewingevent_entry.link) %>
<tr>
<% if @count == 0 %> <!--- print revieweeId,time, round only for the first row-->
Copy link
Collaborator

Choose a reason for hiding this comment

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

Suggested change
<% if @count == 0 %> <!--- print revieweeId,time, round only for the first row-->
<% if @count.zero?%> <!--- print revieweeId,time, round only for the first row-->

<%end%>
</tr>
<% end %>
<% @count=@count+1 %>
Copy link
Collaborator

Choose a reason for hiding this comment

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

use each_with_index to manage counters

<% each_link_detail << individual_link_time %>
<% @per_round_time << each_link_detail %>
<td align="center"><%= sprintf("%.2f",individual_link_time) %></td>
<% if @count == 0 %>
Copy link
Collaborator

Choose a reason for hiding this comment

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

Suggested change
<% if @count == 0 %>
<% if @count.zero? %>

<th align="right">Time per link (mins)</th>
<th></th>
</tr>
<% @reviewers = Participant.where(user_id: @user.id) %>
Copy link
Collaborator

Choose a reason for hiding this comment

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

why are these using the @ symbol

@@ -410,6 +410,30 @@ def get_css_style_for_calibration_report(diff)
css_class
end

def get_time_spent_on_review_for_certain_team_for_each_round(map_id)
Copy link
Collaborator

Choose a reason for hiding this comment

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

this name is too long. also dont use 'get' in ruby

@LeooHsiang
Copy link

We are building on the Previous team's implementation, E1989. Their solution wrote to the database often and did not optimize storage space to the desired metric.

We have implemented a solution by using the built-in function of pstore to mitigate database interaction and reduce the number of entries in the database significantly.

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.

None yet

6 participants