Skip to content

Add error messaging to course team page - #538

Merged
singingwolfboy merged 10 commits into
masterfrom
db/course-team-admin-grants
Aug 1, 2013
Merged

Add error messaging to course team page#538
singingwolfboy merged 10 commits into
masterfrom
db/course-team-admin-grants

Conversation

@singingwolfboy

Copy link
Copy Markdown
Contributor

Hitting "Add User" with no email address, or trying to add a user to the course
team multiple times. Also added confirmation prompt when deleting a user from a course team.

Comment thread cms/templates/manage_users.html Outdated

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

I think the course name should be in quotes. Otherwise the message is very awkward for courses with a space in the name.

@cahrens

cahrens commented Aug 1, 2013

Copy link
Copy Markdown

I'm seeing this message showing up "Promote another member to Admin to remove your admin rights" for users that are not the one logged in.

Repro steps:

  1. Be logged in with a course open for which you have admin rights. NOTE: it turns out I was logged in with an account marked "is_staff". I realize this may change things.
  2. Add another staff person
  3. Make them admin
  4. Revoke your own admin status (should you be able to do this?).
  5. Now the other user you made admin has this message "Promote another member to Admin to remove your admin rights". That message should only be shown when that user is logged in.

When logged in with an account that is NOT marked "is_staff", I see a different (probably more serious) issue. It would be great to have a test for this.

  1. Be logged in with a course for which you have admin rights (but not is_staff).
  2. Add another staff person
  3. Make them admin
  4. Revoke your own admin status (should you be able to do this?).
  5. You get an error message-- error changing user's role (unknown).

@cahrens

cahrens commented Aug 1, 2013

Copy link
Copy Markdown

Update BDD spec for changes (and Jasmine/Lettuce tests as appropriate).

Comment thread cms/templates/manage_users.html Outdated

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

Same thing about course name needing to be quoted.

@cahrens

cahrens commented Aug 1, 2013

Copy link
Copy Markdown

@talbs I have a general style question. Should we use single quotes or double quotes for quoting things? For instance, to quote the name of the course in the error message, should it be "Your New Course" or 'Your New Course'? The error we get back if the e-mail does not exist has the e-mail address in single quotes (Could not find user by e-mail address 'foo@foo.bar'). We should be consistent, but I'm not sure what the correct way is.

Comment thread cms/templates/manage_users.html Outdated

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

Change to CMS.Models.Course. Only using the name field.

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

Can we confirm that the admin rights have been removed? I think all the steps below would pass even if admin rights did not get revoked from the original user.

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

Wrote a new test for that in d8a7901

@cahrens

cahrens commented Aug 1, 2013

Copy link
Copy Markdown

👍 After additional test is done.

@cahrens

cahrens commented Aug 1, 2013

Copy link
Copy Markdown

Love it. 👍

singingwolfboy added a commit that referenced this pull request Aug 1, 2013
Add error messaging to course team page
@singingwolfboy
singingwolfboy merged commit 2ad78d9 into master Aug 1, 2013
@talbs

talbs commented Aug 1, 2013

Copy link
Copy Markdown
Contributor

@cahrens, RE: quotation mark standards - I agree we should be consistent. I think double marks is a fine way to go. To add fuel to the fire, should these characters be transformed into HTML entity-based double quotes (” and “)?

chrisrossi pushed a commit to jazkarta/edx-platform that referenced this pull request Mar 31, 2014
…edit

partial credit for an inputfield

May want to couple this (in the future) with some UI feedback to indicate partial credit was awarded.
smarnach referenced this pull request in open-craft/openedx-platform Oct 28, 2015
caesar2164 pushed a commit to caesar2164/edx-platform that referenced this pull request Nov 30, 2016
* stv/bump/inline-drop-down:
  Bump InlineDropDown XBlock
jfavellar90 pushed a commit to eduNEXT/edx-platform that referenced this pull request Apr 11, 2018
* add changes to development branch

* comments

* remove mig file
jfavellar90 pushed a commit to eduNEXT/edx-platform that referenced this pull request Apr 11, 2018
* removing setting configure and rather setting setting module to lms

* try sys convert

* FIX: key error

* add changes to development branch (openedx#538)

* add changes to development branch

* comments

* remove mig file

* [ci skip] switcching to proversity fork of social auths

* [ci skip] fixing encoding

* [Development] Update clear course cache (openedx#542)

* FIX: key error

* update clear-course-cache to accept multiple courses

* [ci skip] reverting conftest
jfavellar90 pushed a commit to eduNEXT/edx-platform that referenced this pull request Apr 11, 2018
* FIX: key error

* Proversity/development (openedx#546)

* removing setting configure and rather setting setting module to lms

* try sys convert

* FIX: key error

* add changes to development branch (openedx#538)

* add changes to development branch

* comments

* remove mig file

* [ci skip] switcching to proversity fork of social auths

* [ci skip] fixing encoding

* [Development] Update clear course cache (openedx#542)

* FIX: key error

* update clear-course-cache to accept multiple courses

* [ci skip] reverting conftest

* Proversity/development (openedx#558)

ENH: bulk grades api to be granular
ENH: course order by
ADD: harambee custom backend SSO

* dynamically update release dates
dgamanenko referenced this pull request in raccoongang/edx-platform Jun 14, 2018
* add changes to development branch

* comments

* remove mig file
dgamanenko referenced this pull request in raccoongang/edx-platform Jun 14, 2018
* removing setting configure and rather setting setting module to lms

* try sys convert

* FIX: key error

* add changes to development branch (#538)

* add changes to development branch

* comments

* remove mig file

* [ci skip] switcching to proversity fork of social auths

* [ci skip] fixing encoding

* [Development] Update clear course cache (#542)

* FIX: key error

* update clear-course-cache to accept multiple courses

* [ci skip] reverting conftest
dgamanenko referenced this pull request in raccoongang/edx-platform Jun 14, 2018
* FIX: key error

* Proversity/development (#546)

* removing setting configure and rather setting setting module to lms

* try sys convert

* FIX: key error

* add changes to development branch (#538)

* add changes to development branch

* comments

* remove mig file

* [ci skip] switcching to proversity fork of social auths

* [ci skip] fixing encoding

* [Development] Update clear course cache (#542)

* FIX: key error

* update clear-course-cache to accept multiple courses

* [ci skip] reverting conftest

* Proversity/development (#558)

ENH: bulk grades api to be granular
ENH: course order by
ADD: harambee custom backend SSO

* dynamically update release dates
Sujeet1379 pushed a commit to chandrudev/edx-platform that referenced this pull request Nov 17, 2022
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.

3 participants