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

Open native speedgrader from web discussion #3131

Open
wants to merge 5 commits into
base: master
Choose a base branch
from

Conversation

vargaat
Copy link
Collaborator

@vargaat vargaat commented Feb 11, 2025

refs: MBL-17942
affects: Teacher
release note: SpeedGrader now correctly opens from graded discussions.

test plan:

  • Create a graded discussion.
  • Post a comment as a student and as a non-student user.
  • Open the discussion from the teacher app.
  • Open SpeedGrader from the main post of the discussion.
  • SpeedGrader should open.
  • Open SpeedGrader from a post of a student.
  • SpeedGrader should open focused on that student.
  • Open SpeedGrader from a post of a teacher.
  • SpeedGrader should open with a no submissions remark.

Screenshots

BeforeAfter

Checklist

  • Tested on phone
  • Tested on tablet
  • Approve from product

refs: MBL-17942
affects: Teacher
release note: SpeedGrader now correctly opens from graded discussions.

test plan:
- Create a graded discussion.
- Post a comment as a student and as a non-student user.
- Open the discussion from the teacher app.
- Open SpeedGrader from the main post of the discussion.
- SpeedGrader should open.
- Open SpeedGrader from a post of a student.
- SpeedGrader should open focused on that student.
- Open SpeedGrader from a post of a teacher.
- SpeedGrader should open with an error.
@inst-danger
Copy link
Contributor

inst-danger commented Feb 11, 2025

Teacher Build QR Code:

@inst-danger
Copy link
Contributor

inst-danger commented Feb 11, 2025

Release Note:

SpeedGrader now correctly opens from graded discussions.

Affected Apps: Teacher

MBL-17942

Coverage New % Master % Delta
Canvas iOS 91.52% 91.52% 0%

Generated by 🚫 dangerJS against dabcba5

Copy link
Contributor

@suhaibabsi-inst suhaibabsi-inst left a comment

Choose a reason for hiding this comment

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

Code +1

@@ -21,6 +21,7 @@ import UIKit
import Core

class SpeedGraderViewController: ScreenViewTrackableViewController, PagesViewControllerDataSource {
static let AllUsersUserID = "speedgrader"
Copy link
Contributor

Choose a reason for hiding this comment

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

Should it be private ?

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

I'm using it in tests so it can't be private but I added the internal modifier to it to make this usage explicit.

@suhaibabsi-inst
Copy link
Contributor

QA +1

Tested on iPhone, iOS 18.2.0, Teacher app.

@ndrsszsz
Copy link
Contributor

Found this issue. When speedgrader is opened and then we put the app into background and then back to foreground and then we tap on done button, the app crashes.

RPReplay_Final1739361305.MP4

Copy link
Contributor

@ndrsszsz ndrsszsz left a comment

Choose a reason for hiding this comment

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

The previously mentioned crash is probably not related to this PR so I'm approving this.
QA +1

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.

4 participants