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

E2360. View for results of bidding #2659

Open
wants to merge 27 commits into
base: main
Choose a base branch
from

Conversation

Shreshth-Malik
Copy link

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 the Expertiza Bot

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

@Shreshth-Malik Shreshth-Malik changed the title Updated the ruby version in docker file and installed curl in docker_setup E2360. View for results of bidding Oct 30, 2023
Copy link
Member

@efg efg left a comment

Choose a reason for hiding this comment

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

Good job! I like how you have divided functionality and written the code. There are just some naming and capitalization issues to address.

redirect_to controller: 'tree_display', action: 'list'
end

def bidding_details
Copy link
Member

Choose a reason for hiding this comment

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

This name needs to be changed. It gives no hint of what the method does.

<%= label_tag('assignment_form[assignment][is_intelligent]', 'Enable bidding for topics?') %>
<img src="/assets/info.png" title="This feature allow students to &quot;bid&quot; for topics.
Instructor must specify when topics are assigned, by going to the Due Dates tab and
entering a due date for &quot;signup&quot;."/>
<br>
<%= button_to 'View Bidding Details', {:controller => 'lottery', :action => 'bidding_details', :id => @assignment_form.assignment.id},
Copy link
Member

Choose a reason for hiding this comment

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

Again, "details" is not specific enough to give an idea of what will be shown. Would "Show bids by priority" be better? Also, please capitalize only the first word on the button, not all words ... less "shouting", and notice that's how Github does it.

@@ -0,0 +1,59 @@
<h3>Bidding Details for <%= @assignment.name %></h3>
Copy link
Member

Choose a reason for hiding this comment

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

Another occurrence of "Details" ...

<thead>
<tr>
<th>Topic Name</th>
<th>Bidding Teams</th>
Copy link
Member

Choose a reason for hiding this comment

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

"Topic name", "Bidding teams", etc.

@@ -107,7 +107,7 @@ en_US:
mico_task: "Micro Task Assignment?"
review_reviewers: "Reviews visible to all other reviewers?"
calibrate_training: "Calibration for training?"
reputation_algorithm: "Reputation Alogorithm?"
reputation_algorithm: "Reputation Algorithm?"
Copy link
Member

Choose a reason for hiding this comment

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

--> "Reputation algorithm". Notice that the other strings capitalize only the first word.

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

2 participants