-
Notifications
You must be signed in to change notification settings - Fork 192
i18n Issues Mega Issue part 2 (Controversial part) #1922
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
Changes from all commits
342ec62
cd9732a
f85fc33
9bbb40c
d51c741
4ab1399
e22b1d6
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change | ||||
|---|---|---|---|---|---|---|
|
|
@@ -205,7 +205,7 @@ describe('FilesAndUploads', () => { | |||||
| await mockStore(RequestStatus.SUCCESSFUL); | ||||||
| axiosMock.onGet(`${getAssetsUrl(courseId)}?display_name=download.png&page_size=1`).reply(200, { assets: [] }); | ||||||
| axiosMock.onPost(getAssetsUrl(courseId)).reply(200, generateNewAssetApiResponse()); | ||||||
| const addFilesButton = screen.getByLabelText('file-input'); | ||||||
| const addFilesButton = screen.getByLabelText(messages.fileInputAriaLabel.defaultMessage); | ||||||
|
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more.
Suggested change
Nit: This may seem to go against the usual "DRY" developer maxim, but I generally prefer that strings are hard-coded in test files, because it makes it much more clear when things get changed accidentally. When the test references the code, it's easier to introduce a regression without noticing. In this case it's not a big deal though. Feel free to leave it as is.
Contributor
Author
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. I understand your point but lets keep it like this this time since it is not a major thing. |
||||||
| userEvent.upload(addFilesButton, file); | ||||||
| await executeThunk(validateAssetFiles(courseId, [file]), store.dispatch); | ||||||
| await waitFor(() => { | ||||||
|
|
@@ -221,7 +221,7 @@ describe('FilesAndUploads', () => { | |||||
| axiosMock.onGet( | ||||||
| `${getAssetsUrl(courseId)}?display_name=mOckID6&page_size=1`, | ||||||
| ).reply(200, { assets: [{ display_name: 'mOckID6' }] }); | ||||||
| const addFilesButton = screen.getByLabelText('file-input'); | ||||||
| const addFilesButton = screen.getByLabelText(messages.fileInputAriaLabel.defaultMessage); | ||||||
| userEvent.upload(addFilesButton, file); | ||||||
| await executeThunk(validateAssetFiles(courseId, [file]), store.dispatch); | ||||||
| expect(screen.getByText(filesPageMessages.overwriteConfirmMessage.defaultMessage)).toBeVisible(); | ||||||
|
|
@@ -242,7 +242,7 @@ describe('FilesAndUploads', () => { | |||||
| }; | ||||||
|
|
||||||
| axiosMock.onPost(getAssetsUrl(courseId)).reply(200, responseData); | ||||||
| const addFilesButton = screen.getByLabelText('file-input'); | ||||||
| const addFilesButton = screen.getByLabelText(messages.fileInputAriaLabel.defaultMessage); | ||||||
| userEvent.upload(addFilesButton, file); | ||||||
| await executeThunk(validateAssetFiles(courseId, [file]), store.dispatch); | ||||||
|
|
||||||
|
|
@@ -266,7 +266,7 @@ describe('FilesAndUploads', () => { | |||||
| axiosMock.onGet( | ||||||
| `${getAssetsUrl(courseId)}?display_name=mOckID6&page_size=1`, | ||||||
| ).reply(200, { assets: [{ display_name: 'mOckID6' }] }); | ||||||
| const addFilesButton = screen.getByLabelText('file-input'); | ||||||
| const addFilesButton = screen.getByLabelText(messages.fileInputAriaLabel.defaultMessage); | ||||||
| userEvent.upload(addFilesButton, file); | ||||||
| await executeThunk(validateAssetFiles(courseId, [file]), store.dispatch); | ||||||
|
|
||||||
|
|
@@ -558,7 +558,7 @@ describe('FilesAndUploads', () => { | |||||
| await mockStore(RequestStatus.SUCCESSFUL); | ||||||
| axiosMock.onGet(`${getAssetsUrl(courseId)}?display_name=download.png&page_size=1`).reply(200, { assets: [] }); | ||||||
| axiosMock.onPost(getAssetsUrl(courseId)).reply(413, { error: errorMessage }); | ||||||
| const addFilesButton = screen.getByLabelText('file-input'); | ||||||
| const addFilesButton = screen.getByLabelText(messages.fileInputAriaLabel.defaultMessage); | ||||||
| userEvent.upload(addFilesButton, file); | ||||||
| await waitFor(() => { | ||||||
| const addStatus = store.getState().assets.addingStatus; | ||||||
|
|
@@ -571,7 +571,7 @@ describe('FilesAndUploads', () => { | |||||
| it('404 validation should show error', async () => { | ||||||
| await mockStore(RequestStatus.SUCCESSFUL); | ||||||
| axiosMock.onGet(`${getAssetsUrl(courseId)}?display_name=download.png&page_size=1`).reply(404); | ||||||
| const addFilesButton = screen.getByLabelText('file-input'); | ||||||
| const addFilesButton = screen.getByLabelText(messages.fileInputAriaLabel.defaultMessage); | ||||||
| userEvent.upload(addFilesButton, file); | ||||||
| await executeThunk(addAssetFile(courseId, file, 1), store.dispatch); | ||||||
| const addStatus = store.getState().assets.addingStatus; | ||||||
|
|
@@ -584,7 +584,7 @@ describe('FilesAndUploads', () => { | |||||
| await mockStore(RequestStatus.SUCCESSFUL); | ||||||
| axiosMock.onGet(`${getAssetsUrl(courseId)}?display_name=download.png&page_size=1`).reply(200, { assets: [] }); | ||||||
| axiosMock.onPost(getAssetsUrl(courseId)).reply(404); | ||||||
| const addFilesButton = screen.getByLabelText('file-input'); | ||||||
| const addFilesButton = screen.getByLabelText(messages.fileInputAriaLabel.defaultMessage); | ||||||
| userEvent.upload(addFilesButton, file); | ||||||
| await executeThunk(addAssetFile(courseId, file, 1), store.dispatch); | ||||||
| const addStatus = store.getState().assets.addingStatus; | ||||||
|
|
||||||
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.
Just curious, why are you replacing the ternary operators?
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 one was a suggestion from a fellow, so the code looks cleaner and more readable.