ylqjgm
/
canvas-lms
mirror of https://github.com/instructure/canvas-lms.git
1
0
Fork 0
Code Issues Packages Projects Releases Wiki Activity

refactor grading standards controller 'index' action 

refactor the grading standards controller and put
permissions in place.

closes CNVS-18223

test plan:
1. While logged in as an admin, go to the grading standards
  account page (/accounts/:account_id/grading_standards) and create
  a couple grading standards (it will probably make your life
  easier to put the word "Admin" in the title of each
  grading standard you create).
2. Log in as a teacher in a course that belongs to
  the account used in step 1. go to the grading standards
  page (/courses/:course_id/grading_standards) and create
  a couple grading standards (it will probably make your life
  easier to put the word "Teacher" in the title of each
  grading standard you create)
3. Log in as a TA in a course that belongs to
  the account used in step 1. go to the grading standards
  page (/courses/:course_id/grading_standards) and create
  a couple grading standards (it will probably make your life
  easier to put the word "TA" in the title of each
  grading standard you create)
4. Now that you've created all the grading standards, ensure
  that the following permissions are in place when logged in
  at different access levels:

Logged in as admin:
  - At the COURSE url (/courses/:course_id/grading_standards)
    - Can view Grading Standards created at the following levels
      -Admin
      -Teacher
      -TA
    - Can edit/delete Grading Standards created at the following levels
      -Admin
      -Teacher
      -TA
  - At the ACCOUNT url (/accounts/:account_id/grading_standards)
    - Can view Grading Standards created at the following levels
      -Admin
    - Can edit/delete Grading Standards created at the following levels
      -Admin

Logged in as teacher:
  - At the COURSE url (/courses/:course_id/grading_standards)
    - Can view Grading Standards created at the following levels
      -Admin
      -Teacher
      -TA
    - Can edit/delete Grading Standards created at the following levels
      -Teacher
      -TA
  - Permission should be denied at the ACCOUNT url
    (/accounts/:account_id/grading_standards)

Logged in as TA:
  - At the COURSE url (/courses/:course_id/grading_standards)
    - Can view Grading Standards created at the following levels
      -Admin
      -Teacher
      -TA
    - Can edit/delete Grading Standards created at the following levels
      -Teacher
      -TA
  - Permission should be denied at the ACCOUNT url
    (/accounts/:account_id/grading_standards)

5. Now, create a sub-account under the account you used in steps 1-4.
  Create an admin for the new sub-account, and create a new Course
  under the sub-account. Publish the course, and create a Teacher
  and a TA for the new course.
6. Just as you did in steps 1-3, create grading standards using the
  new sub-account admin, teacher, and TA.
7. Ensure that the following permissions are in place when logged in
  at different access levels:

Logged in as sub-account admin:
  - At the NEW COURSE url (/courses/:new_course_id/grading_standards)
    - Can view the Grading Standards created at the following levels
      -Admin
      -Sub-Account Admin
      -New teacher
      -New TA
    - Can edit/delete Grading Standards created at the following levels
      -Sub-Account Admin
      -New Teacher
      -New TA
  - At the SUB-ACCOUNT url (/accounts/:sub_account_id/grading_standards)
    - Can view Grading Standards created at the following levels
      -Admin
      -Sub-Account Admin
    - Can edit/delete Grading Standards created at the following levels
      -Sub-Account Admin
  - Permission should be denied at the ACCOUNT url
    (/accounts/:account_id/grading_standards)

Logged in as new teacher:
  - At the NEW COURSE url (/courses/:new_course_id/grading_standards)
    - Can view Grading Standards created at the following levels
      -Admin
      -Sub-Account Admin
      -New Teacher
      -New TA
    - Can edit/delete Grading Standards created at the following levels
      -New Teacher
      -New TA
  - Permission should be denied at the SUB-ACCOUNT url
    (/accounts/:sub_account_id/grading_standards)
  - Permission should be denied at the ACCOUNT url
    (/accounts/:account_id/grading_standards)

Logged in as new TA:
  - At the NEW COURSE url (/courses/:new_course_id/grading_standards)
    - Can view Grading Standards created at the following levels
      -Admin
      -Sub-Account Admin
      -New Teacher
      -New TA
    - Can edit/delete Grading Standards created at the following levels
      -New Teacher
      -New TA
  - Permission should be denied at the SUB-ACCOUNT url
    (/accounts/:sub_account_id/grading_standards)
  - Permission should be denied at the ACCOUNT url
    (/accounts/:account_id/grading_standards)

Change-Id: I483f5b516f5786a669e7316af80ae382873cf9d1
Reviewed-on: https://gerrit.instructure.com/48109
Reviewed-by: Cameron Sutter <csutter@instructure.com>
QA-Review: Amber Taniuchi <amber@instructure.com>
Tested-by: Jenkins
Product-Review: Spencer Olson <solson@instructure.com>
This commit is contained in:
Spencer Olson 2015-01-29 20:32:00 -07:00
parent 05f10c15b3
commit 2bab15c2cd
7 changed files with 246 additions and 85 deletions

12
app/controllers/grading_standards_controller.rb
View File

@ -36,12 +36,8 @@ class GradingStandardsController < ApplicationController
respond_to do |format|
format.html { }
format.json {
editable_ids = @standards.select{ |s| s.context_type == @context.class.name &&
s.context_id == @context.id }.map(&:id)
standards_json = @standards.map do |s|
standard = s.as_json(methods: [:display_name, :context_code, :assessed_assignment?])
standard[:can_edit_and_destroy] = editable_ids.include?(s.id)
standard
s.as_json(methods: [:display_name, :context_code, :assessed_assignment?, :context_name], permissions: {user: @current_user})
end
render :json => standards_json
}
@ -56,7 +52,7 @@ class GradingStandardsController < ApplicationController
@standard.user = @current_user
respond_to do |format|
if @standard.save
format.json{ render :json => @standard }
format.json{ render :json => @standard.as_json(permissions: {user: @current_user}) }
else
format.json{ render :json => @standard.errors, :status => :bad_request }
end
@ -70,7 +66,7 @@ class GradingStandardsController < ApplicationController
@standard.user = @current_user
respond_to do |format|
if @standard.update_attributes(params[:grading_standard])
format.json{ render :json => @standard }
format.json{ render :json => @standard.as_json(permissions: {user: @current_user}) }
else
format.json{ render :json => @standard.errors, :status => :bad_request }
end
@ -83,7 +79,7 @@ class GradingStandardsController < ApplicationController
if authorized_action(@context, @current_user, :manage_grades)
respond_to do |format|
if @standard.destroy
format.json{ render :json => @standard }
format.json{ render :json => @standard.as_json(permissions: {user: @current_user}) }
else
format.json{ render :json => @standard.errors, :status => :bad_request }
end

65
app/jsx/grading/grading_standard.jsx
View File

@ -13,9 +13,9 @@ function(React, DataRow, $, I18n) {
getInitialState: function() {
return {
standard: this.props.standard,
permissions: this.props.permissions,
editingStandard: $.extend(true, {}, this.props.standard),
editing: this.props.editing,
can_edit_and_destroy: this.props.can_edit_and_destroy,
saving: false,
justAdded: this.props.justAdded
};
@ -24,9 +24,9 @@ function(React, DataRow, $, I18n) {
componentWillReceiveProps: function(nextProps) {
this.setState({
standard: nextProps.standard,
permissions: nextProps.permissions,
editingStandard: $.extend(true, {}, this.props.standard),
editing: nextProps.editing,
can_edit_and_destroy: nextProps.can_edit_and_destroy,
saving: nextProps.saving,
justAdded: nextProps.justAdded
});
@ -78,7 +78,7 @@ function(React, DataRow, $, I18n) {
.success(function(response){
self.setState({standard: response.grading_standard, editing: false,
editingStandard: $.extend(true, {}, response.grading_standard),
can_edit_and_destroy: true, saving: false, justAdded: false});
saving: false, justAdded: false});
})
.error(function(){
self.setState({saving: false});
@ -90,11 +90,6 @@ function(React, DataRow, $, I18n) {
return this.state.standard && this.state.standard["assessed_assignment?"];
},
renderIdNames: function() {
if(this.assessedAssignment()) return "grading_standard_blank";
return "grading_standard_" + (this.state.standard ? this.state.standard.id : "blank");
},
deleteDataRow: function(index) {
this.state.editingStandard.data.splice(index, 1);
this.setState({editingStandard: this.state.editingStandard});
@ -111,6 +106,36 @@ function(React, DataRow, $, I18n) {
this.setState({editingStandard: this.state.editingStandard});
},
changeRowMinScore: function(index, newMinVal) {
this.state.editingStandard.data[index][1] = newMinVal;
this.setState({editingStandard: this.state.editingStandard});
},
changeRowName: function(index, newRowName) {
this.state.editingStandard.data[index][0] = newRowName;
this.setState({editingStandard: this.state.editingStandard});
},
renderCannotManageMessage: function() {
if(this.state.standard.context_name){
return (
<div>
{I18n.t("(%{context}: %{contextName})", { context: this.state.standard.context_type.toLowerCase(), contextName: this.state.standard.context_name })}
</div>
);
}
return (
<div>
{I18n.t("(%{context} level)", { context: this.state.standard.context_type.toLowerCase() })}
</div>
);
},
renderIdNames: function() {
if(this.assessedAssignment()) return "grading_standard_blank";
return "grading_standard_" + (this.state.standard ? this.state.standard.id : "blank");
},
renderTitle: function() {
if(this.state.editing){
return (
@ -131,22 +156,14 @@ function(React, DataRow, $, I18n) {
);
},
changeRowMinScore: function(index, newMinVal) {
this.state.editingStandard.data[index][1] = newMinVal;
this.setState({editingStandard: this.state.editingStandard});
},
changeRowName: function(index, newRowName) {
this.state.editingStandard.data[index][0] = newRowName;
this.setState({editingStandard: this.state.editingStandard});
},
renderDataRows: function() {
var data = this.state.editing ? this.state.editingStandard.data : this.state.standard.data;
return data.map(function(item, idx, array){
return (<DataRow key={idx} row={item} siblingRow={array[idx - 1]} editing={this.state.editing}
onDeleteRow={this.deleteDataRow} onInsertRow={this.insertGradingStandardRow}
onRowMinScoreChange={this.changeRowMinScore} onRowNameChange={this.changeRowName}/>);
return (
<DataRow key={idx} row={item} siblingRow={array[idx - 1]} editing={this.state.editing}
onDeleteRow={this.deleteDataRow} onInsertRow={this.insertGradingStandardRow}
onRowMinScoreChange={this.changeRowMinScore} onRowNameChange={this.changeRowName}/>
);
}, this);
},
@ -194,7 +211,7 @@ function(React, DataRow, $, I18n) {
},
renderIconsAndTitle: function() {
if(this.state.can_edit_and_destroy){
if(this.state.permissions.manage){
return (
<div>
{this.renderTitle()}
@ -216,8 +233,8 @@ function(React, DataRow, $, I18n) {
<i className="icon-edit standalone-icon"/>
<i className="icon-trash standalone-icon"/>
</div>
<div className="pull-left account-level-notification">
{I18n.t("(account level)")}
<div className="pull-left cannot-manage-notification">
{this.renderCannotManageMessage()}
</div>
</div>
);

97
app/jsx/grading/tab_container.jsx
View File

@ -21,8 +21,10 @@ function(React, GradingStandard, GradingPeriod, $, I18n, _) {
$.getJSON(ENV.GRADING_STANDARDS_URL)
.done(this.gotStandards)
$.getJSON(ENV.GRADING_PERIODS_URL)
.done(this.gotPeriods)
if(ENV.MULTIPLE_GRADING_PERIODS){
$.getJSON(ENV.GRADING_PERIODS_URL)
.done(this.gotPeriods)
}
},
gotStandards: function(standards) {
@ -52,7 +54,6 @@ function(React, GradingStandard, GradingPeriod, $, I18n, _) {
})
.success(function(newStandard) {
newStandard.editing = true;
newStandard.can_edit_and_destroy = true;
newStandard.justAdded = true;
newStandards.unshift(newStandard);
$(this).slideDown();
@ -98,6 +99,51 @@ function(React, GradingStandard, GradingPeriod, $, I18n, _) {
});
},
hasAdminOrTeacherRole: function() {
return _.intersection(ENV.current_user_roles, ["teacher", "admin"]).length > 0;
},
renderAddGradingStandardButton: function() {
if(this.hasAdminOrTeacherRole()){
return(
<div className="rs-margin-all pull-right">
<a href="#" onClick={this.addGradingStandard} className="btn pull-right add_standard_link">
<i className="icon-add"/>
{I18n.t(" Add grading scheme")}
</a>
</div>
);
}
return null;
},
renderGradingStandards: function() {
if(!this.state.standards){
return null;
} else if(this.state.standards.length === 0){
return <h3>{I18n.t("No grading schemes to display")}</h3>;
}
var self = this;
return this.state.standards.map(function(s){
return (<GradingStandard key={s.grading_standard.id} standard={s.grading_standard}
editing={!!s.editing} permissions={s.grading_standard.permissions}
justAdded={!!s.justAdded} onDeleteGradingStandard={self.deleteGradingStandard}
onDeleteGradingStandardNoWarning={self.deleteGradingStandardNoWarning}/>);
});
},
renderGradingPeriods: function() {
if(!this.state.periods){
return null;
} else if(this.state.periods.length === 0){
return <h3>{I18n.t("No grading periods to display")}</h3>;
}
return this.state.periods.map(function(p){
return (<GradingPeriod key={p.id} title={p.title} startDate={new Date(p.start_date)}
endDate={new Date(p.end_date)} weight={p.weight}/>);
});
},
render: function () {
if(ENV.MULTIPLE_GRADING_PERIODS){
return (
@ -113,14 +159,9 @@ function(React, GradingStandard, GradingPeriod, $, I18n, _) {
</div>
</div>
<div id="grading-standards-tab">
<div className="rs-margin-all pull-right">
<a href="#" onClick={this.addGradingStandard} className="btn pull-right add_standard_link">
<i className="icon-add"/>
{I18n.t(" Add grading scheme")}
</a>
</div>
{this.renderAddGradingStandardButton()}
<div id="standards" className="content-box react_grading_standards">
{this.renderStandards()}
{this.renderGradingStandards()}
</div>
</div>
</div>
@ -130,45 +171,13 @@ function(React, GradingStandard, GradingPeriod, $, I18n, _) {
return (
<div>
<h1 tabIndex="0">{I18n.t("Grading Schemes")}</h1>
<div className="rs-margin-all pull-right">
<a href="#" onClick={this.addGradingStandard} className="btn pull-right add_standard_link">
<i className="icon-add"/>
{I18n.t(" Add grading scheme")}
</a>
</div>
{this.renderAddGradingStandardButton()}
<div id="standards" className="content-box react_grading_standards">
{this.renderStandards()}
{this.renderGradingStandards()}
</div>
</div>
);
};
},
renderStandards: function() {
if(!this.state.standards){
return null;
} else if(this.state.standards.length === 0){
return <h3>{I18n.t("No grading standards to display")}</h3>;
}
var self = this;
return this.state.standards.map(function(s){
return (<GradingStandard key={s.grading_standard.id} standard={s.grading_standard}
editing={!!s.editing} can_edit_and_destroy={s.can_edit_and_destroy}
justAdded={!!s.justAdded} onDeleteGradingStandard={self.deleteGradingStandard}
onDeleteGradingStandardNoWarning={self.deleteGradingStandardNoWarning}/>);
});
},
renderGradingPeriods: function() {
if(!this.state.periods){
return null;
} else if(this.state.periods.length === 0){
return <h3>{I18n.t("No grading periods to display")}</h3>;
}
return this.state.periods.map(function(p){
return (<GradingPeriod key={p.id} title={p.title} startDate={new Date(p.start_date)}
endDate={new Date(p.end_date)} weight={p.weight}/>);
});
}
});

16
app/models/account_user.rb
View File

@ -81,11 +81,11 @@ class AccountUser < ActiveRecord::Base
p.dispatch :new_account_user
p.to {|record| record.account.users}
p.whenever {|record| record.just_created }
p.dispatch :account_user_registration
p.to {|record| record.user }
p.whenever {|record| @account_user_registration }
p.dispatch :account_user_notification
p.to {|record| record.user }
p.whenever {|record| @account_user_notification }
@ -99,13 +99,13 @@ class AccountUser < ActiveRecord::Base
def readable_type
AccountUser.readable_type(self.role.name)
end
def account_user_registration!
@account_user_registration = true
self.save!
@account_user_registration = false
end
def account_user_notification!
@account_user_notification = true
self.save!
@ -154,21 +154,21 @@ class AccountUser < ActiveRecord::Base
type
end
end
def self.any_for?(user)
!account_ids_for_user(user).empty?
end
def self.account_ids_for_user(user)
@account_ids_for ||= {}
@account_ids_for[user.id] ||= Rails.cache.fetch(['account_ids_for_user', user].cache_key) do
AccountUser.for_user(user).map(&:account_id)
end
end
def self.for_user_and_account?(user, account_id)
account_ids_for_user(user).include?(account_id)
end
scope :for_user, lambda { |user| where(:user_id => user) }
end

9
app/models/grading_standard.rb
View File

@ -64,6 +64,11 @@ class GradingStandard < ActiveRecord::Base
VERSION = 2
set_policy do
given { |user| self.context.grants_right?(user, :manage) }
can :manage
end
def version
read_attribute(:version).presence || 1
end
@ -154,6 +159,10 @@ class GradingStandard < ActiveRecord::Base
self.assignments.active.joins(:submissions).where("submissions.workflow_state='graded'").exists?
end
def context_name
self.context.name
end
def update_data(params)
self.data = params.to_a.sort_by{|_, lower_bound| lower_bound}.reverse
end

2
app/stylesheets/pages/shared/_grading_standards.sass
View File

@ -67,7 +67,7 @@
.disabled-links
float: right
opacity: 0.5
.account-level-notification
.cannot-manage-notification
font-weight: normal
margin-left: 4px
.icon-edit

130
spec/models/grading_standard_spec.rb
View File

@ -263,4 +263,134 @@ describe GradingStandard do
end
end
end
describe "permissions:" do
context "course belonging to root account" do
before(:once) do
@root_account = Account.default
@sub_account = @root_account.sub_accounts.create!
course_with_teacher_logged_in(account: @root_account)
@enrollment.update_attributes(workflow_state: "active")
@root_account_standard = grading_standard_for(@root_account)
@sub_account_standard = grading_standard_for(@sub_account)
@course_standard = grading_standard_for(@course)
end
context "root-account admin" do
before(:once) do
account_admin_user(account: @root_account)
end
it "should be able to manage root-account level grading standards" do
expect(@root_account_standard.grants_right?(@admin, :manage)).to eq(true)
end
it "should be able to manage sub-account level grading standards" do
expect(@sub_account_standard.grants_right?(@admin, :manage)).to eq(true)
end
it "should be able to manage course level grading standards" do
expect(@course_standard.grants_right?(@admin, :manage)).to eq(true)
end
end
context "sub-account admin" do
before(:once) do
account_admin_user(account: @sub_account)
end
it "should NOT be able to manage root-account level grading standards" do
expect(@root_account_standard.grants_right?(@admin, :manage)).to eq(false)
end
it "should be able to manage sub-account level grading standards" do
expect(@sub_account_standard.grants_right?(@admin, :manage)).to eq(true)
end
it "should NOT be able to manage course level grading standards, when the course is under the root-account" do
expect(@course_standard.grants_right?(@admin, :manage)).to eq(false)
end
end
context "teacher" do
it "should NOT be able to manage root-account level grading standards" do
expect(@root_account_standard.grants_right?(@teacher, :manage)).to eq(false)
end
it "should NOT be able to manage sub-account level grading standards" do
expect(@sub_account_standard.grants_right?(@teacher, :manage)).to eq(false)
end
it "should be able to manage course level grading standards" do
expect(@course_standard.grants_right?(@teacher, :manage)).to eq(true)
end
end
end
context "course belonging to sub-account" do
before(:once) do
@root_account = Account.default
@sub_account = @root_account.sub_accounts.create!
course_with_teacher_logged_in(account: @sub_account)
@enrollment.update_attributes(workflow_state: "active")
@root_account_standard = grading_standard_for(@root_account)
@sub_account_standard = grading_standard_for(@sub_account)
@course_standard = grading_standard_for(@course)
end
context "root-account admin" do
before(:once) do
account_admin_user(account: @root_account)
end
it "should be able to manage root-account level grading standards" do
expect(@root_account_standard.grants_right?(@admin, :manage)).to eq(true)
end
it "should be able to manage sub-account level grading standards" do
expect(@sub_account_standard.grants_right?(@admin, :manage)).to eq(true)
end
it "should be able to manage course level grading standards" do
expect(@course_standard.grants_right?(@admin, :manage)).to eq(true)
end
end
context "sub-account admin" do
before(:once) do
account_admin_user(account: @sub_account)
end
it "should NOT be able to manage root-account level grading standards" do
expect(@root_account_standard.grants_right?(@admin, :manage)).to eq(false)
end
it "should be able to manage sub-account level grading standards" do
expect(@sub_account_standard.grants_right?(@admin, :manage)).to eq(true)
end
it "should be able to manage course level grading standards, when the course is under the sub-account" do
expect(@course_standard.grants_right?(@admin, :manage)).to eq(true)
end
end
context "teacher" do
it "should NOT be able to manage root-account level grading standards" do
expect(@root_account_standard.grants_right?(@teacher, :manage)).to eq(false)
end
it "should NOT be able to manage sub-account level grading standards" do
expect(@sub_account_standard.grants_right?(@teacher, :manage)).to eq(false)
end
it "should be able to manage course level grading standards" do
expect(@course_standard.grants_right?(@teacher, :manage)).to eq(true)
end
end
end
end
end