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

feat(KUI-1135): add fallback without link when SU is course department #278

Merged
2 commits merged into from
Feb 2, 2024
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
5 changes: 4 additions & 1 deletion public/js/app/components/CourseSectionList.jsx
Original file line number Diff line number Diff line change
Expand Up @@ -132,7 +132,10 @@ function CourseSectionList(props) {

function getOther() {
const prepare = [
{ header: translation.courseInformation.course_department, text: course.course_department_link },
{
header: translation.courseInformation.course_department,
text: course.course_department_link ?? course.course_department,
},
{
header: translation.courseInformation.course_main_subject,
text: course.course_main_subject,
Expand Down
33 changes: 33 additions & 0 deletions public/js/app/components/__tests__/CourseSectionList.test.js
Original file line number Diff line number Diff line change
Expand Up @@ -107,4 +107,37 @@ describe('Component <CourseSectionList>', () => {
syllabusText = screen.getByText(syllabusLiteratureComment, { exact: false })
expect(syllabusText).toBeInTheDocument()
})

test('renders course department correctly', () => {
const courseDepartmentLinkText = 'Link text'
const courseDepartmentFallbackValue = 'Fallback text'

const courseInfoWithLink = {
course_department_link: `<a href="/itm/" target="blank">${courseDepartmentLinkText}</a>`,
course_department: courseDepartmentFallbackValue,
}
const { rerender } = render(
<WebContextProvider configIn={{ courseInfo: courseInfoWithLink }}>
<CourseSectionList courseInfo={courseInfoWithLink} />
</WebContextProvider>
)
const linkText1 = screen.queryByText(courseDepartmentLinkText)
expect(linkText1).toBeInTheDocument()
const fallbackText1 = screen.queryByText(courseDepartmentFallbackValue)
expect(fallbackText1).not.toBeInTheDocument()

const courseInfoWithoutLink = {
course_department_link: undefined,
course_department: courseDepartmentFallbackValue,
}
rerender(
<WebContextProvider configIn={{ courseInfo: courseInfoWithoutLink }}>
<CourseSectionList courseInfo={courseInfoWithoutLink} />
</WebContextProvider>
)
const linkText2 = screen.queryByText(courseDepartmentLinkText)
expect(linkText2).not.toBeInTheDocument()
const fallbackText2 = screen.queryByText(courseDepartmentFallbackValue)
expect(fallbackText2).toBeInTheDocument()
})
})
10 changes: 2 additions & 8 deletions server/controllers/courseCtrl.js
Original file line number Diff line number Diff line change
Expand Up @@ -22,6 +22,7 @@ const {
MAX_1_MONTH,
MAX_2_MONTH,
} = require('../util/constants')
const { buildCourseDepartmentLink } = require('../util/courseDepartmentUtils')
const { formatVersionDate, getDateFormat } = require('../util/dates')
const i18n = require('../../i18n')

Expand Down Expand Up @@ -72,14 +73,7 @@ function _parseCourseDefaultInformation(courseDetails, language) {
course_contact_name: parceContactName(course.infoContactName, language),
course_department: parseOrSetEmpty(course.department.name, language),
course_department_code: parseOrSetEmpty(course.department.code, language),
course_department_link:
parseOrSetEmpty(course.department.name, language) !== INFORM_IF_IMPORTANT_INFO_IS_MISSING[language]
? '<a href="/' +
course.department.name.split('/')[0].toLowerCase() +
'/" target="blank">' +
course.department.name +
'</a>'
: INFORM_IF_IMPORTANT_INFO_IS_MISSING[language],
course_department_link: buildCourseDepartmentLink(course.department, language),
course_disposition: parseOrSetEmpty(course.courseDeposition, language),
course_education_type_id: course.educationalTypeId || null,
course_examiners: INFORM_IF_IMPORTANT_INFO_IS_MISSING[language],
Expand Down
36 changes: 36 additions & 0 deletions server/util/__tests__/courseDepartmentUtils.test.js
Original file line number Diff line number Diff line change
@@ -0,0 +1,36 @@
const { buildCourseDepartmentLink } = require('../courseDepartmentUtils')
const { INFORM_IF_IMPORTANT_INFO_IS_MISSING } = require('../constants')

function htmlStringToElement(html) {
Copy link
Contributor

Choose a reason for hiding this comment

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

💯

const template = document.createElement('template')
template.innerHTML = html
const htmlElement = template.content.children[0]
return htmlElement
}

describe('course department link utils', () => {
test('returns department link', () => {
const department = { code: 'JH', name: 'EECS/Datavetenskap' }
const result = buildCourseDepartmentLink(department, 'en')
expect(result).toBeDefined()

const element = htmlStringToElement(result)
expect(element.href).toBe('/eecs/')
expect(element.innerHTML).toBe('EECS/Datavetenskap')
})

test('returns undefined for Stockholm university as department', () => {
const department = { code: 'UL', name: 'Stockholms universitet' }

const result = buildCourseDepartmentLink(department, 'en')
expect(result).not.toBeDefined()
})

test.each([undefined, { name: undefined }])('returns fallback text if department is %p', department => {
Copy link
Contributor

Choose a reason for hiding this comment

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

🥇 I had no idea this was possible \o/

const resultSv = buildCourseDepartmentLink(department, 'sv')
expect(resultSv).toBe(INFORM_IF_IMPORTANT_INFO_IS_MISSING['sv'])

const resultEn = buildCourseDepartmentLink(department, 'en')
expect(resultEn).toBe(INFORM_IF_IMPORTANT_INFO_IS_MISSING['en'])
})
})
8 changes: 5 additions & 3 deletions server/util/constants.js
Original file line number Diff line number Diff line change
@@ -1,19 +1,21 @@
const PROGRAMME_URL = '/student/kurser/program'
const INFORM_IF_IMPORTANT_INFO_IS_MISSING = {
en: '<i>No information inserted</i>',
sv: '<i>Ingen information tillagd</i>'
sv: '<i>Ingen information tillagd</i>',
}
const INFORM_IF_IMPORTANT_INFO_IS_MISSING_ABOUT_MIN_FIELD_OF_STUDY = {
en: 'This course does not belong to any Main field of study.',
sv: 'Denna kurs tillhör inget huvudområde.'
sv: 'Denna kurs tillhör inget huvudområde.',
}
const MAX_1_MONTH = 3
const MAX_2_MONTH = 9
const DEPARTMENT_CODE_STOCKHOLM_UNIVERSITY = 'UL'

module.exports = {
PROGRAMME_URL,
INFORM_IF_IMPORTANT_INFO_IS_MISSING,
INFORM_IF_IMPORTANT_INFO_IS_MISSING_ABOUT_MIN_FIELD_OF_STUDY,
MAX_1_MONTH,
MAX_2_MONTH
MAX_2_MONTH,
DEPARTMENT_CODE_STOCKHOLM_UNIVERSITY,
}
22 changes: 22 additions & 0 deletions server/util/courseDepartmentUtils.js
Original file line number Diff line number Diff line change
@@ -0,0 +1,22 @@
const { INFORM_IF_IMPORTANT_INFO_IS_MISSING, DEPARTMENT_CODE_STOCKHOLM_UNIVERSITY } = require('./constants')

function isDepartmentStockholmUniversity(courseDepartment) {
return courseDepartment.code === DEPARTMENT_CODE_STOCKHOLM_UNIVERSITY
}

function buildCourseDepartmentLink(courseDepartment, language) {
Copy link
Contributor

Choose a reason for hiding this comment

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

Much more readable!

if (!courseDepartment?.name) {
return INFORM_IF_IMPORTANT_INFO_IS_MISSING[language]
}

if (isDepartmentStockholmUniversity(courseDepartment)) {
return undefined
}

const departmentLinkPart = courseDepartment.name.split('/')[0].toLowerCase()
return `<a href="/${departmentLinkPart}/" target="blank">${courseDepartment.name}</a>`
}

module.exports = {
buildCourseDepartmentLink,
}
Loading