-
Notifications
You must be signed in to change notification settings - Fork 172
Frontend MVP for Source Academy Rook #1878
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
Frontend MVP for Source Academy Rook #1878
Conversation
…y configurations
…kend. Yarn format
…pe routes toLowerCase
… were previously commented out already
…er to use postCourseConfig saga
…anguage on first render
…be hardcoded to assessement types
Accidentally merged in the previous PR ==. Reissuing it here |
assessmentOverview.category === 'Mission' || assessmentOverview.category === 'Path' | ||
assessmentOverview.type === 'Missions' || assessmentOverview.type === 'Paths' |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
probably need the achievements team to remove this hardcode
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This may need a new assessment flag: "Show in achievements?"
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Will be removing this categorisation as discussed. Probably in #1886
if (data.length + users.length > 1000) { | ||
setInvalidCsvMsg('Please limit each upload to 1000 entries!'); | ||
hasInvalidInput = true; | ||
} |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
still hardcoded to 1000 per upload for now. will be relooked when the super user rights are implemented
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Looks good mostly, just a few minor issues
assessmentOverview.category === 'Mission' || assessmentOverview.category === 'Path' | ||
assessmentOverview.type === 'Missions' || assessmentOverview.type === 'Paths' |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This may need a new assessment flag: "Show in achievements?"
? numGraded === 0 | ||
? 'none' | ||
: numGraded === numQuestions | ||
? 'graded' | ||
: 'grading' | ||
: 'excluded'; | ||
|
||
const courseId: () => string = () => { | ||
const id = store.getState().session.courseId; |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This makes me very very sad...
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Ok but in all seriousness, we can merge for now but when we get the course ID in the route, it would be good to pass it in as a parameter rather than accessing a global singleton like that
src/pages/academy/adminPanel/subcomponents/assessmentConfigPanel/BooleanCell.tsx
Outdated
Show resolved
Hide resolved
src/pages/academy/adminPanel/subcomponents/assessmentConfigPanel/DeleteRowCell.tsx
Outdated
Show resolved
Hide resolved
src/pages/academy/adminPanel/subcomponents/assessmentConfigPanel/NumericCell.tsx
Outdated
Show resolved
Hide resolved
I left quite a few comments but I think the main issues are
|
…Selector in Welcome Page. Fix miscellaneous typos
6031a71
to
be5920b
Compare
be5920b
to
47a4b6e
Compare
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Thanks for all the work!
Description
Frontend MVP for Source Academy Rook
Status:
To get feedback on UIFrontend mockups not updated yet'Paths' logic is yet to be refactoredAssessment 'Grade' has not been fully removedTo relook at course 'viewability' for admins (it is currently disabled for all when viewable == false)Note: To temporarily disable the create course button for deployment
Changelog (Knight --> Rook):
Added multitenant functionality
Updated frontend routing
Updates to setting of course configuration
New admin panel for course admins
Changes to testcase types
'Grade' metric has been deprecated. Assessments will now solely use XP for grading.
Type of change
How to test
To be tested with the new backend (source-academy/backend#775)
Checklist