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

fix: xblock error mfe unit preview #1508

Merged
merged 9 commits into from
Nov 1, 2024
1 change: 1 addition & 0 deletions .env
Original file line number Diff line number Diff line change
Expand Up @@ -48,3 +48,4 @@ TWITTER_HASHTAG=''
TWITTER_URL=''
USER_INFO_COOKIE_NAME=''
OPTIMIZELY_FULL_STACK_SDK_KEY=''
SHOW_UNGRADED_ASSIGNMENT_PROGRESS=''
1 change: 1 addition & 0 deletions .env.development
Original file line number Diff line number Diff line change
Expand Up @@ -50,3 +50,4 @@ SESSION_COOKIE_DOMAIN='localhost'
CHAT_RESPONSE_URL='http://localhost:18000/api/learning_assistant/v1/course_id'
PRIVACY_POLICY_URL='http://localhost:18000/privacy'
OPTIMIZELY_FULL_STACK_SDK_KEY=''
SHOW_UNGRADED_ASSIGNMENT_PROGRESS=''
1 change: 1 addition & 0 deletions .env.test
Original file line number Diff line number Diff line change
Expand Up @@ -47,3 +47,4 @@ TWITTER_HASHTAG='myedxjourney'
TWITTER_URL='https://twitter.com/edXOnline'
USER_INFO_COOKIE_NAME='edx-user-info'
PRIVACY_POLICY_URL='http://localhost:18000/privacy'
SHOW_UNGRADED_ASSIGNMENT_PROGRESS=''
13 changes: 3 additions & 10 deletions .github/workflows/validate.yml
Original file line number Diff line number Diff line change
Expand Up @@ -9,21 +9,16 @@ on:
jobs:
tests:
runs-on: ubuntu-latest
strategy:
matrix:
node: [18, 20]
steps:
- uses: actions/checkout@v4
- uses: actions/setup-node@v4
with:
node-version: ${{ matrix.node }}
node-version-file: '.nvmrc'
- run: make validate.ci
- name: Archive code coverage results
uses: actions/upload-artifact@v4
with:
name: code-coverage-report-${{ matrix.node }}
# When we're only using Node 20, replace the line above with the following:
# name: code-coverage-report
name: code-coverage-report
path: coverage/*.*
coverage:
runs-on: ubuntu-latest
Expand All @@ -33,9 +28,7 @@ jobs:
- name: Download code coverage results
uses: actions/download-artifact@v4
with:
name: code-coverage-report-20
# When we're only using Node 20, replace the line above with the following:
# name: code-coverage-report
name: code-coverage-report
- name: Upload coverage
uses: codecov/codecov-action@v4
with:
Expand Down
117 changes: 115 additions & 2 deletions src/course-home/progress-tab/ProgressTab.test.jsx
Original file line number Diff line number Diff line change
@@ -1,6 +1,6 @@
import React from 'react';
import { Factory } from 'rosie';
import { getConfig } from '@edx/frontend-platform';
import { getConfig, setConfig } from '@edx/frontend-platform';
import { sendTrackEvent } from '@edx/frontend-platform/analytics';
import { getAuthenticatedHttpClient } from '@edx/frontend-platform/auth';
import { breakpoints } from '@openedx/paragon';
Expand Down Expand Up @@ -545,6 +545,111 @@ describe('Progress Tab', () => {
await fetchAndRender();
expect(screen.getByText('Grades & Credit')).toBeInTheDocument();
});

it('does not render ungraded subsections when SHOW_UNGRADED_ASSIGNMENT_PROGRESS is false', async () => {
// The second assignment has has_graded_assignment set to false, so it should not be shown.
setTabData({
section_scores: [
{
display_name: 'First section',
subsections: [
{
assignment_type: 'Homework',
block_key: 'block-v1:edX+DemoX+Demo_Course+type@sequential+block@12345',
display_name: 'First subsection',
learner_has_access: true,
has_graded_assignment: true,
num_points_earned: 1,
num_points_possible: 2,
percent_graded: 1.0,
show_correctness: 'always',
show_grades: true,
url: 'http://learning.edx.org/course/course-v1:edX+Test+run/first_subsection',
},
],
},
{
display_name: 'Second section',
subsections: [
{
assignment_type: 'Homework',
display_name: 'Second subsection',
learner_has_access: true,
has_graded_assignment: false,
num_points_earned: 1,
num_points_possible: 1,
percent_graded: 1.0,
show_correctness: 'always',
show_grades: true,
url: 'http://learning.edx.org/course/course-v1:edX+Test+run/second_subsection',
},
],
},
],
});

await fetchAndRender();
expect(screen.getByText('First subsection')).toBeInTheDocument();
expect(screen.queryByText('Second subsection')).not.toBeInTheDocument();
});

it('renders both graded and ungraded subsections when SHOW_UNGRADED_ASSIGNMENT_PROGRESS is true', async () => {
// The second assignment has has_graded_assignment set to false.
setConfig({
...getConfig(),
SHOW_UNGRADED_ASSIGNMENT_PROGRESS: true,
});

setTabData({
section_scores: [
{
display_name: 'First section',
subsections: [
{
assignment_type: 'Homework',
block_key: 'block-v1:edX+DemoX+Demo_Course+type@sequential+block@12345',
display_name: 'First subsection',
learner_has_access: true,
has_graded_assignment: true,
num_points_earned: 1,
num_points_possible: 2,
percent_graded: 1.0,
show_correctness: 'always',
show_grades: true,
url: 'http://learning.edx.org/course/course-v1:edX+Test+run/first_subsection',
},
],
},
{
display_name: 'Second section',
subsections: [
{
assignment_type: 'Homework',
display_name: 'Second subsection',
learner_has_access: true,
has_graded_assignment: false,
num_points_earned: 1,
num_points_possible: 1,
percent_graded: 1.0,
show_correctness: 'always',
show_grades: true,
url: 'http://learning.edx.org/course/course-v1:edX+Test+run/second_subsection',
},
],
},
],
});

await fetchAndRender();
expect(screen.getByText('First subsection')).toBeInTheDocument();
expect(screen.getByText('Second subsection')).toBeInTheDocument();

// reset config for other tests
setConfig({
...getConfig(),
SHOW_UNGRADED_ASSIGNMENT_PROGRESS: false,
});
});
});

describe('Grade Summary', () => {
Expand Down Expand Up @@ -809,7 +914,7 @@ describe('Progress Tab', () => {

// Open the problem score drawer
fireEvent.click(problemScoreDrawerToggle);
expect(screen.getByText('Problem Scores:')).toBeInTheDocument();
expect(screen.getAllByText('Graded Scores:').length).toBeGreaterThan(1);
expect(screen.getAllByText('0/1')).toHaveLength(3);
});

Expand All @@ -821,6 +926,14 @@ describe('Progress Tab', () => {
expect(screen.getByText('Detailed grades')).toBeInTheDocument();
expect(screen.getByText('You currently have no graded problem scores.')).toBeInTheDocument();
});

it('renders Detailed Grades table when section scores are populated', async () => {
await fetchAndRender();
expect(screen.getByText('Detailed grades')).toBeInTheDocument();

expect(screen.getByText('First subsection'));
expect(screen.getByText('Second subsection'));
});
});

describe('Certificate Status', () => {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -7,6 +7,7 @@ import { FormattedMessage, injectIntl, intlShape } from '@edx/frontend-platform/
import { Blocked } from '@openedx/paragon/icons';
import { Icon, Hyperlink } from '@openedx/paragon';
import { useModel } from '../../../../generic/model-store';
import { showUngradedAssignments } from '../../utils';

import DetailedGradesTable from './DetailedGradesTable';

Expand All @@ -28,6 +29,8 @@ const DetailedGrades = ({ intl }) => {
} = useModel('progress', courseId);

const hasSectionScores = sectionScores.length > 0;
const emptyTableMsg = showUngradedAssignments()
? messages.detailedGradesEmpty : messages.detailedGradesEmptyOnlyGraded;

const logOutlineLinkClick = () => {
sendTrackEvent('edx.ui.lms.course_progress.detailed_grades.course_outline_link.clicked', {
Expand All @@ -54,7 +57,25 @@ const DetailedGrades = ({ intl }) => {

return (
<section className="text-dark-700">
<h3 className="h4 mb-3">{intl.formatMessage(messages.detailedGrades)}</h3>
<h3 className="h4">{intl.formatMessage(messages.detailedGrades)}</h3>
<ul className="micro mb-3 pl-3 text-gray-700">
<li>
<b>{intl.formatMessage(messages.practiceScoreLabel)} </b>
<FormattedMessage
id="progress.detailedGrades.practice-label.info.text"
defaultMessage="Scores from non-graded activities meant for practice and self-assessment."
description="Information text about non-graded practice score label"
/>
</li>
<li>
<b>{intl.formatMessage(messages.gradedScoreLabel)} </b>
<FormattedMessage
id="progress.detailedGrades.problem-label.info.text"
defaultMessage="Scores from activities that contribute to your final grade."
description="Information text about graded problem score label"
/>
</li>
</ul>
{gradesFeatureIsPartiallyLocked && (
<div className="mb-3 small ml-0 d-inline">
<Icon className="mr-1 mt-1 d-inline-flex" style={{ height: '1rem', width: '1rem' }} src={Blocked} data-testid="blocked-icon" />
Expand All @@ -65,9 +86,9 @@ const DetailedGrades = ({ intl }) => {
<DetailedGradesTable />
)}
{!hasSectionScores && (
<p className="small">{intl.formatMessage(messages.detailedGradesEmpty)}</p>
<p className="small">{intl.formatMessage(emptyTableMsg)}</p>
)}
{overviewTabUrl && (
{overviewTabUrl && !showUngradedAssignments() && (
<p className="x-small m-0">
<FormattedMessage
id="progress.ungradedAlert"
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -9,6 +9,7 @@ import { DataTable } from '@openedx/paragon';
import { useModel } from '../../../../generic/model-store';
import messages from '../messages';
import SubsectionTitleCell from './SubsectionTitleCell';
import { showUngradedAssignments } from '../../utils';

const DetailedGradesTable = ({ intl }) => {
const {
Expand All @@ -24,9 +25,10 @@ const DetailedGradesTable = ({ intl }) => {
sectionScores.map((chapter) => {
const subsectionScores = chapter.subsections.filter(
(subsection) => !!(
subsection.hasGradedAssignment
&& subsection.showGrades
&& (subsection.numPointsPossible > 0 || subsection.numPointsEarned > 0)),
(showUngradedAssignments() || subsection.hasGradedAssignment)
&& subsection.showGrades
&& (subsection.numPointsPossible > 0 || subsection.numPointsEarned > 0)
),
);

if (subsectionScores.length === 0) {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -10,9 +10,12 @@ import messages from '../messages';

const ProblemScoreDrawer = ({ intl, problemScores, subsection }) => {
const isLocaleRtl = isRtl(getLocale());

const scoreLabel = subsection.hasGradedAssignment ? messages.gradedScoreLabel : messages.practiceScoreLabel;

return (
<span className="row w-100 m-0 x-small ml-4 pt-2 pl-1 text-gray-700 flex-nowrap">
<span id="problem-score-label" className="col-auto p-0">{intl.formatMessage(messages.problemScoreLabel)}</span>
<span id="problem-score-label" className="col-auto p-0">{intl.formatMessage(scoreLabel)}</span>
<div className={classNames('col', 'p-0', { 'greyed-out': !subsection.learnerHasAccess })}>
<ul className="list-unstyled row w-100 m-0" aria-labelledby="problem-score-label">
{problemScores.map((problemScore, i) => (
Expand All @@ -31,7 +34,10 @@ ProblemScoreDrawer.propTypes = {
earned: PropTypes.number.isRequired,
possible: PropTypes.number.isRequired,
})).isRequired,
subsection: PropTypes.shape({ learnerHasAccess: PropTypes.bool }).isRequired,
subsection: PropTypes.shape({
learnerHasAccess: PropTypes.bool,
hasGradedAssignment: PropTypes.bool,
}).isRequired,
};

export default injectIntl(ProblemScoreDrawer);
16 changes: 13 additions & 3 deletions src/course-home/progress-tab/grades/messages.ts
Original file line number Diff line number Diff line change
Expand Up @@ -91,9 +91,14 @@ const messages = defineMessages({
defaultMessage: 'Detailed grades',
description: 'Headline for the (detailed grade) section in the progress tab',
},
detailedGradesEmpty: {
detailedGradesEmptyOnlyGraded: {
id: 'progress.detailedGrades.emptyTable',
defaultMessage: 'You currently have no graded problem scores.',
description: 'It indicate that there are no graded problem or assignments to be scored',
},
detailedGradesEmpty: {
id: 'progress.detailedGrades.including-ungraded.emptyTable',
defaultMessage: 'You currently have no graded or ungraded problem scores.',
description: 'It indicate that there are no problem or assignments to be scored',
},
footnotesTitle: {
Expand Down Expand Up @@ -158,11 +163,16 @@ const messages = defineMessages({
defaultMessage: 'Passing grade',
description: 'Label for mark on the (grade bar) chart which indicate the poisition of passing grade on the bar',
},
problemScoreLabel: {
gradedScoreLabel: {
id: 'progress.detailedGrades.problemScore.label',
defaultMessage: 'Problem Scores:',
defaultMessage: 'Graded Scores:',
description: 'Label text which precedes detailed view of all scores per assignment',
},
practiceScoreLabel: {
id: 'progress.detailedGrades.practice.problemScore.label',
defaultMessage: 'Practice Scores:',
description: 'Label text which precedes detailed view of all ungraded problem scores per assignment',
},
problemScoreToggleAltText: {
id: 'progress.detailedGrades.problemScore.toggleButton',
defaultMessage: 'Toggle individual problem scores for {subsectionTitle}',
Expand Down
7 changes: 7 additions & 0 deletions src/course-home/progress-tab/utils.ts
Original file line number Diff line number Diff line change
@@ -0,0 +1,7 @@
import { getConfig } from '@edx/frontend-platform';

/* eslint-disable import/prefer-default-export */
export const showUngradedAssignments = () => (
getConfig().SHOW_UNGRADED_ASSIGNMENT_PROGRESS === 'true'
|| getConfig().SHOW_UNGRADED_ASSIGNMENT_PROGRESS === true
);
2 changes: 1 addition & 1 deletion src/courseware/CoursewareContainer.jsx
Original file line number Diff line number Diff line change
Expand Up @@ -168,7 +168,7 @@ class CoursewareContainer extends Component {

checkFetchSequence = memoize((sequenceId) => {
if (sequenceId) {
this.props.fetchSequence(sequenceId);
this.props.fetchSequence(sequenceId, this.props.isPreview);
}
});

Expand Down
3 changes: 2 additions & 1 deletion src/courseware/course/Course.jsx
Original file line number Diff line number Diff line change
Expand Up @@ -34,6 +34,7 @@ const Course = ({
celebrations,
isStaff,
isNewDiscussionSidebarViewEnabled,
originalUserIsStaff,
} = useModel('courseHomeMeta', courseId);
const sequence = useModel('sequences', sequenceId);
const section = useModel('sections', sequence ? sequence.sectionId : null);
Expand All @@ -42,7 +43,7 @@ const Course = ({
const navigate = useNavigate();
const { pathname } = useLocation();

if (!isStaff && pathname.startsWith('/preview')) {
if (!originalUserIsStaff && pathname.startsWith('/preview')) {
const courseUrl = pathname.replace('/preview', '');
navigate(courseUrl, { replace: true });
}
Expand Down
2 changes: 1 addition & 1 deletion src/courseware/course/sequence/Sequence.jsx
Original file line number Diff line number Diff line change
Expand Up @@ -204,7 +204,7 @@ const Sequence = ({
sequenceId={sequenceId}
unitId={unitId}
unitLoadedHandler={handleUnitLoaded}
isStaff={isStaff}
isOriginalUserStaff={originalUserIsStaff}
/>
{unitHasLoaded && renderUnitNavigation(false)}
</div>
Expand Down
Loading
Loading