Plagiarism checks: Use file extensions for plagiarism view
#9350
Conversation
github-actions
bot
added
tests
server
WalkthroughThe changes involve the removal and renaming of several constants and methods across various components and services within the application. Key modifications include the elimination of the Changes
Sequence Diagram(s)sequenceDiagram
participant User
participant TextSubmissionViewerComponent
participant FileService
User->>TextSubmissionViewerComponent: Request file for plagiarism view
TextSubmissionViewerComponent->>FileService: Check file extension
alt If text file
FileService-->>TextSubmissionViewerComponent: Fetch file content
else If binary file
TextSubmissionViewerComponent-->>User: Mark as binary
end
Thank you for using CodeRabbit. We offer it for free to the OSS community and would appreciate your support in helping us grow. If you find it useful, would you consider giving us a shout-out on your favorite social media? 🪧 TipsChatThere are 3 ways to chat with CodeRabbit:
Note: Be mindful of the bot's finite context window. It's strongly recommended to break down tasks such as reading entire modules into smaller chunks. For a focused discussion, use review comments to chat about specific files and their changes, instead of using the PR comments. CodeRabbit Commands (Invoked using PR comments)
Other keywords and placeholders
Documentation and Community
|
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 5
Review details
Configuration used: .coderabbit.yaml
Review profile: ASSERTIVE
Files selected for processing (11)
- src/main/java/de/tum/cit/aet/artemis/programming/service/RepositoryService.java (0 hunks)
- src/main/webapp/app/exercises/programming/shared/code-editor/file-browser/code-editor-file-browser.component.ts (2 hunks)
- src/main/webapp/app/exercises/programming/shared/code-editor/file-browser/supported-file-extensions.ts (0 hunks)
- src/main/webapp/app/exercises/programming/shared/code-editor/service/code-editor-repository.service.ts (0 hunks)
- src/main/webapp/app/exercises/shared/plagiarism/plagiarism-split-view/text-submission-viewer/text-submission-viewer.component.ts (2 hunks)
- src/main/webapp/app/lecture/lecture-attachments.component.ts (2 hunks)
- src/main/webapp/app/lecture/lecture-unit/lecture-unit-management/attachment-unit-form/attachment-unit-form.component.ts (2 hunks)
- src/main/webapp/app/lecture/lecture-update.component.ts (2 hunks)
- src/main/webapp/app/shared/constants/file-extensions.constants.ts (2 hunks)
- src/main/webapp/app/shared/http/file-uploader.service.ts (2 hunks)
- src/test/javascript/spec/component/plagiarism/text-submission-viewer.component.spec.ts (2 hunks)
Files not reviewed due to no reviewable changes (3)
- src/main/java/de/tum/cit/aet/artemis/programming/service/RepositoryService.java
- src/main/webapp/app/exercises/programming/shared/code-editor/file-browser/supported-file-extensions.ts
- src/main/webapp/app/exercises/programming/shared/code-editor/service/code-editor-repository.service.ts
Additional context used
Path-based instructions (8)
src/main/webapp/app/exercises/programming/shared/code-editor/file-browser/code-editor-file-browser.component.ts (1)
src/main/webapp/app/exercises/shared/plagiarism/plagiarism-split-view/text-submission-viewer/text-submission-viewer.component.ts (1)
src/main/webapp/app/lecture/lecture-attachments.component.ts (1)
src/main/webapp/app/lecture/lecture-unit/lecture-unit-management/attachment-unit-form/attachment-unit-form.component.ts (1)
src/main/webapp/app/lecture/lecture-update.component.ts (1)
src/main/webapp/app/shared/constants/file-extensions.constants.ts (1)
src/main/webapp/app/shared/http/file-uploader.service.ts (1)
src/test/javascript/spec/component/plagiarism/text-submission-viewer.component.spec.ts (1)
Pattern
src/test/javascript/spec/**/*.ts: jest: true; mock: NgMocks; bad_practices: avoid_full_module_import; perf_improvements: mock_irrelevant_deps; service_testing: mock_http_for_logic; no_schema: avoid_NO_ERRORS_SCHEMA; expectation_specificity: true; solutions: {boolean: toBeTrue/False, reference: toBe, existence: toBeNull/NotNull, undefined: toBeUndefined, class_obj: toContainEntries/toEqual, spy_calls: {not_called: not.toHaveBeenCalled, once: toHaveBeenCalledOnce, with_value: toHaveBeenCalledWith|toHaveBeenCalledExactlyOnceWith}}
Biome
src/main/webapp/app/exercises/programming/shared/code-editor/file-browser/code-editor-file-browser.component.ts
[error] 585-585: Forbidden non-null assertion.
(lint/style/noNonNullAssertion)
src/main/webapp/app/exercises/shared/plagiarism/plagiarism-split-view/text-submission-viewer/text-submission-viewer.component.ts
[error] 178-178: Forbidden non-null assertion.
(lint/style/noNonNullAssertion)
src/main/webapp/app/lecture/lecture-attachments.component.ts
[error] 39-39: Template literals are preferred over string concatenation.
Unsafe fix: Use a template literal.
(lint/style/useTemplate)
src/main/webapp/app/lecture/lecture-unit/lecture-unit-management/attachment-unit-form/attachment-unit-form.component.ts
[error] 44-44: Template literals are preferred over string concatenation.
Unsafe fix: Use a template literal.
(lint/style/useTemplate)
src/main/webapp/app/lecture/lecture-update.component.ts
[error] 51-51: Template literals are preferred over string concatenation.
Unsafe fix: Use a template literal.
(lint/style/useTemplate)
Additional comments not posted (30)
src/main/webapp/app/shared/http/file-uploader.service.ts (2)
5-5: LGTM!The import statement has been correctly updated to reflect the renaming of the constant. The new name
UPLOAD_MARKDOWN_FILE_EXTENSIONSprovides more clarity about its purpose related to uploads.
17-17: LGTM!The
acceptedMarkdownFileExtensionsproperty has been correctly updated to reference the renamedUPLOAD_MARKDOWN_FILE_EXTENSIONSconstant. This change is consistent with the renaming of the constant and ensures that the property uses the correct value.src/main/webapp/app/lecture/lecture-unit/lecture-unit-management/attachment-unit-form/attachment-unit-form.component.ts (1)
6-6: LGTM!The import statement has been correctly updated to use the
UPLOAD_FILE_EXTENSIONSconstant, which aligns with the PR objective.src/main/webapp/app/shared/constants/file-extensions.constants.ts (4)
4-6: LGTM!The renaming of the constant and the updated comment improve clarity and reflect the change in the server-side definition source. The changes adhere to the coding guidelines.
11-13: LGTM!The renaming of the constant and the updated comment improve clarity and reflect the change in the server-side definition source. The changes adhere to the coding guidelines.
48-344: LGTM!The addition of the
TEXT_FILE_EXTENSIONSconstant provides a clear distinction between file extensions allowed for uploads and those readable in a file editor. The comment provides clear guidelines on the format and interpretation of the file extensions, and the list is comprehensive. The changes adhere to the coding guidelines.
Line range hint
1-344: Overall approvalThe changes in this file improve clarity, reflect updates in the server-side definitions, and introduce a new category for text file extensions. The constants are well-documented and adhere to the coding guidelines. The file serves a clear purpose, and the changes enhance its functionality and maintainability. No additional issues or concerns were identified during the review.
src/main/webapp/app/lecture/lecture-update.component.ts (2)
15-15: LGTM!The import statement change clarifies the purpose of the file extensions related to uploads.
49-49: LGTM!The
allowedFileExtensionsproperty update is consistent with the import statement change and clarifies the purpose of the allowed file extensions.src/main/webapp/app/lecture/lecture-attachments.component.ts (2)
12-12: LGTM!The import statement change clarifies the purpose of the constant.
37-37: LGTM!The property update is consistent with the updated import statement.
src/main/webapp/app/exercises/shared/plagiarism/plagiarism-split-view/text-submission-viewer/text-submission-viewer.component.ts (4)
14-14: LGTM!The import statement follows the correct syntax and naming conventions. The purpose of the imported constant is clear.
178-195: The file type determination logic looks good!The code correctly determines if the selected file is a text file by checking its extension against the predefined
TEXT_FILE_EXTENSIONSconstant. Fetching the file content only for text files is a good optimization. ThebinaryFileandloadingflags are set appropriately to provide a good user experience.Tools
Biome
[error] 178-178: Forbidden non-null assertion.
(lint/style/noNonNullAssertion)
182-191: File content fetching and processing looks good!The code correctly uses the
repositoryServiceto fetch the file content for text files. The fetched content is then processed by inserting match tokens. Appropriate error handling is in place to set the loading flag to false if there's an issue fetching the file.
192-195: Binary file handling looks good!The code correctly handles the case when the selected file is not a text file. It marks the file as binary by setting
binaryFileto true, which will help display an appropriate message to the user. Settingloadingto false is also correct as no content is being loaded for binary files.src/test/javascript/spec/component/plagiarism/text-submission-viewer.component.spec.ts (14)
Line range hint
48-58: LGTM!The test case is correctly mocking the service method and verifying the expected behavior.
Line range hint
60-70: LGTM!The test case is correctly mocking the service method and verifying the expected behavior.
Line range hint
72-80: LGTM!The test case is correctly mocking the service method and verifying the expected behavior when
hideContentis true.
Line range hint
82-91: LGTM!The test case is correctly mocking the service method to throw an error and verifying the expected behavior.
Line range hint
93-125: LGTM!The test case is correctly mocking the service method with an unordered file structure and verifying the expected behavior. It's correctly setting up the
matchesand verifying the expected sorting and filtering offiles.
Line range hint
127-131: LGTM!The test case is correctly verifying the expected behavior of the
filterFilesmethod.
Line range hint
133-147: LGTM!The test case is correctly mocking the service method and verifying the expected behavior of the
handleFileSelectmethod.
Line range hint
149-163: LGTM!The test case is correctly mocking the service method and verifying the expected behavior of the
handleFileSelectmethod for a binary file.
Line range hint
165-191: LGTM!The test case is correctly mocking the
getMatchesForCurrentFilemethod and verifying the expected behavior of theinsertMatchTokensmethod for exact matches.
Line range hint
193-219: LGTM!The test case is correctly mocking the
getMatchesForCurrentFilemethod and verifying the expected behavior of theinsertMatchTokensmethod for full line matches.
Line range hint
221-230: LGTM!The test case is correctly mocking the
getMatchesForCurrentFilemethod and verifying the expected behavior of theinsertMatchTokensmethod when no matches are present. It's correctly verifying that the HTML tags are escaped in the returned file content.
Line range hint
232-259: LGTM!The test case is correctly mocking the
getMatchesForCurrentFilemethod and verifying the expected behavior of theinsertMatchTokensmethod for exact matches with HTML tags. It's correctly verifying that the matches are wrapped with the appropriate tags and the HTML tags are escaped in the returned file content.
Line range hint
261-288: LGTM!The test case is correctly mocking the
getMatchesForCurrentFilemethod and verifying the expected behavior of theinsertMatchTokensmethod for full line matches with HTML tags. It's correctly verifying that the full lines with matches are wrapped with the appropriate tags and the HTML tags are escaped in the returned file content.
Line range hint
290-315: LGTM!The test case is correctly mocking the
getMatchesForCurrentFilemethod and verifying the expected behavior of theinsertMatchTokenssrc/main/webapp/app/exercises/programming/shared/code-editor/file-browser/code-editor-file-browser.component.ts (1)
29-29: LGTM!The import statement is valid and follows the coding guidelines. The change aligns with the PR objective of enhancing the detection of binary files during plagiarism checks by utilizing a list of file extensions.
.../lecture-unit/lecture-unit-management/attachment-unit-form/attachment-unit-form.component.ts
Show resolved
Hide resolved
.../plagiarism/plagiarism-split-view/text-submission-viewer/text-submission-viewer.component.ts
Show resolved
Hide resolved
.../exercises/programming/shared/code-editor/file-browser/code-editor-file-browser.component.ts
Show resolved
Hide resolved
magaupp
temporarily deployed
to
artemis-test2.artemis.cit.tum.de
GitHub Actions
Inactive
github-actions
bot
added
the
programming
…ions # Conflicts: # src/main/webapp/app/exercises/programming/shared/code-editor/file-browser/supported-file-extensions.ts
magaupp
dismissed stale reviews from coderabbitai[bot], raffifasaro, ole-ve, JohannesStoehr, and SimonEntholzer
via
bdf97b5
magaupp
requested review from
SimonEntholzer,
JohannesStoehr,
ole-ve and
raffifasaro
magaupp
temporarily deployed
to
artemis-test2.artemis.cit.tum.de
GitHub Actions
Inactive
Checklist
General
Server
Client
Changes affecting Programming Exercises
Motivation and Context
The detection of binary files involves Java's
Files.probeContentType()where the results are sometimes guessed from the filename. The results can not reliably detect source code files.Description
This PR the list of file extensions used by the code editor for the detection of binary files. This also makes one REST call unnecessary.
Steps for Testing
Prerequisites:
Binary file not rendered.is shownTestserver States
Note
These badges show the state of the test servers.
Green = Currently available, Red = Currently locked
Click on the badges to get to the test servers.
Review Progress
Performance Review
Code Review
Manual Tests
Test Coverage
Screenshots
Summary by CodeRabbit
Release Notes
New Features
Bug Fixes
Refactor
Tests