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

Programming exercises: Fix translation issue for participation modes in exam management #9293

Conversation

iyannsch
Copy link
Contributor

@iyannsch iyannsch commented Sep 6, 2024

Checklist

General

Client

  • Important: I implemented the changes with a very good performance, prevented too many (unnecessary) REST calls and made sure the UI is responsive, even with large data (e.g. using paging).
  • I strictly followed the principle of data economy for all client-server REST calls.
  • I strictly followed the client coding and design guidelines.
  • Following the theming guidelines, I specified colors only in the theming variable files and checked that the changes look consistent in both the light and the dark theme.
  • I added multiple integration tests (Jest) related to the features (with a high test coverage), while following the test guidelines.
  • I added authorities to all new routes and checked the course groups for displaying navigation elements (links, buttons).
  • I documented the TypeScript code using JSDoc style.
  • I added multiple screenshots/screencasts of my UI changes.
  • I translated all newly inserted strings into English and German.

Changes affecting Programming Exercises

  • High priority: I tested all changes and their related features with all corresponding user types on a test server configured with the integrated lifecycle setup (LocalVC and LocalCI).
  • I tested all changes and their related features with all corresponding user types on a test server configured with Gitlab and Jenkins.

Motivation and Context

As discovered by @SimonEntholzer translations for the Participation Modes were faulty in the exam management. This PR fixes the translations and also hides informations about the Online IDE when it's not enabled on the respective server.

Description

I reused the implementation for the (correctly working and dev-approved) non-exam exercise management components and applied it to exams. As I touched the code anyways, I added a new check that only shows the Online IDE informations when the respective profile is active.

Steps for Testing

Exam Mode Testing

Prerequisites:

  • 1 Instructor
  • 1 Exam with Exercise Groups and Programming Exercises
  1. Navigate to the exercise group in the exam
  2. Verify that the Participation Mode is well readable and no translation errors are shown

Testserver 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

  • I (as a reviewer) confirm that the client changes (in particular related to REST calls and UI responsiveness) are implemented with a very good performance even for very large courses with more than 2000 students.
  • I (as a reviewer) confirm that the server changes (in particular related to database calls) are implemented with a very good performance even for very large courses with more than 2000 students.

Code Review

  • Code Review 1
  • Code Review 2

Manual Tests

  • Test 1
  • Test 2

Exam Mode Test

  • Test 1
  • Test 2

Performance Tests

  • Test 1
  • Test 2

Test Coverage

Screenshots

Bugged view as reported by @SimonEntholzer
image

New, fixed version on a server without THEIA profile enabled
image

New, fixed version on a server with THEIA profile enabled
image

Summary by CodeRabbit

  • New Features

    • Enhanced display logic for programming exercise settings, improving clarity on offline and online editor options.
    • Introduced conditional rendering for the online IDE feature, ensuring it only appears when relevant.
    • Added support for a new online IDE profile, allowing for a more versatile user experience.
  • Bug Fixes

    • Improved responsiveness of the programming exercise management interface by preventing unnecessary rendering of UI elements.

@iyannsch iyannsch self-assigned this Sep 6, 2024
@iyannsch iyannsch requested a review from a team as a code owner September 6, 2024 19:04
Copy link

coderabbitai bot commented Sep 6, 2024

Walkthrough

The changes involve enhancements to the programming exercise management components, specifically focusing on the integration of an online IDE feature. Updates were made to HTML templates for improved display logic and conditional rendering based on the availability of the online IDE. Additionally, new profile constants and boolean properties were introduced in the TypeScript files to manage the state of the online IDE functionality effectively. The testing setup was also updated to reflect these changes.

Changes

Files Change Summary
src/main/webapp/app/exam/manage/exercise-groups/programming-exercise-cell/...html Improved display logic for programming exercise settings; added conditional rendering for online IDE.
src/main/webapp/app/exam/manage/exercise-groups/programming-exercise-cell/...ts Added PROFILE_THEIA constant; introduced onlineIdeEnabled property to manage online IDE state.
src/main/webapp/app/exercises/programming/manage/programming-exercise.component.html Conditional rendering for online IDE section based on onlineIdeEnabled.
src/main/webapp/app/exercises/programming/manage/programming-exercise.component.ts Added PROFILE_THEIA constant; introduced onlineIdeEnabled property to track online IDE status.
src/test/javascript/spec/component/exam/manage/exercise-groups/...spec.ts Enhanced tests for ProgrammingExerciseGroupCellComponent to include PROFILE_THEIA and updated assertions.

Tip

New features

Walkthrough comment now includes:

  • Possibly related PRs: A list of potentially related PRs to help you recall past context.
  • Suggested labels: CodeRabbit can now suggest labels by learning from your past PRs. You can also provide custom labeling instructions in the UI or configuration file.

Notes:

  • Please share any feedback in the discussion post on our Discord.
  • Possibly related PRs, automatic label suggestions based on past PRs, learnings, and possibly related issues require data opt-in (enabled by default).

Recent review details

Configuration used: .coderabbit.yaml
Review profile: ASSERTIVE

Commits

Files that changed from the base of the PR and between e757eff and 65abd00.

Files selected for processing (1)
  • src/test/javascript/spec/component/exam/manage/exercise-groups/programming-exercise-group-cell.component.spec.ts (3 hunks)
Additional context used
Path-based instructions (1)
src/test/javascript/spec/component/exam/manage/exercise-groups/programming-exercise-group-cell.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}}

Additional comments not posted (5)
src/test/javascript/spec/component/exam/manage/exercise-groups/programming-exercise-group-cell.component.spec.ts (5)

14-14: Review the import of ArtemisSharedCommonModule.

The addition of ArtemisSharedCommonModule is appropriate if it provides shared functionalities or components necessary for the tests. Ensure that this module does not introduce unnecessary overhead or dependencies that are not used in the tests.


15-15: Check the use of PROFILE_THEIA constant.

The addition of PROFILE_THEIA to the activeProfiles array is crucial for testing the component's behavior under specific profile conditions. This change aligns with the PR's objective to handle features based on active profiles, ensuring that the tests reflect real-world usage scenarios.


48-48: Validate the profile service mock setup.

The mock setup for ProfileService now includes activeProfiles with PROFILE_THEIA, which is essential for testing the component's conditional rendering based on the profile. This setup is a good practice as it ensures that the component behaves as expected when the profile is active.


53-53: Assess the test module configuration.

The inclusion of ArtemisTestModule and ArtemisSharedCommonModule in the imports section is necessary to provide the required Angular and shared functionalities for the component's testing environment. This configuration is standard and helps isolate the component during testing.


99-99: Examine the assertions for localized strings.

The test cases have been updated to check for specific localized strings, which is a significant improvement over the previous generic boolean outputs. This change ensures that the component renders the correct text based on the active profile, which is crucial for verifying the UI's correctness under different configurations.

Also applies to: 103-103, 107-107


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?

Share
Tips

Chat

There are 3 ways to chat with CodeRabbit:

  • Review comments: Directly reply to a review comment made by CodeRabbit. Example:
    • I pushed a fix in commit <commit_id>.
    • Generate unit testing code for this file.
    • Open a follow-up GitHub issue for this discussion.
  • Files and specific lines of code (under the "Files changed" tab): Tag @coderabbitai in a new review comment at the desired location with your query. Examples:
    • @coderabbitai generate unit testing code for this file.
    • @coderabbitai modularize this function.
  • PR comments: Tag @coderabbitai in a new PR comment to ask questions about the PR branch. For the best results, please provide a very specific query, as very limited context is provided in this mode. Examples:
    • @coderabbitai generate interesting stats about this repository and render them as a table.
    • @coderabbitai show all the console.log statements in this repository.
    • @coderabbitai read src/utils.ts and generate unit testing code.
    • @coderabbitai read the files in the src/scheduler package and generate a class diagram using mermaid and a README in the markdown format.
    • @coderabbitai help me debug CodeRabbit configuration file.

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)

  • @coderabbitai pause to pause the reviews on a PR.
  • @coderabbitai resume to resume the paused reviews.
  • @coderabbitai review to trigger an incremental review. This is useful when automatic reviews are disabled for the repository.
  • @coderabbitai full review to do a full review from scratch and review all the files again.
  • @coderabbitai summary to regenerate the summary of the PR.
  • @coderabbitai resolve resolve all the CodeRabbit review comments.
  • @coderabbitai configuration to show the current CodeRabbit configuration for the repository.
  • @coderabbitai help to get help.

Other keywords and placeholders

  • Add @coderabbitai ignore anywhere in the PR description to prevent this PR from being reviewed.
  • Add @coderabbitai summary to generate the high-level summary at a specific location in the PR description.
  • Add @coderabbitai or @coderabbitai title anywhere in the PR title to generate the title automatically.

Documentation and Community

  • Visit our Documentation for detailed information on how to use CodeRabbit.
  • Join our Discord Community to get help, request features, and share feedback.
  • Follow us on X/Twitter for updates and announcements.

Copy link

@coderabbitai coderabbitai bot left a comment

Choose a reason for hiding this comment

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

Actionable comments posted: 2

@github-actions github-actions bot added deployment-error Added by deployment workflows if an error occured and removed deploy:artemis-test3 labels Sep 6, 2024
@SimonEntholzer SimonEntholzer added deploy:artemis-test3 and removed deployment-error Added by deployment workflows if an error occured labels Sep 6, 2024
@ls1intum ls1intum deleted a comment from github-actions bot Sep 6, 2024
@SimonEntholzer SimonEntholzer temporarily deployed to artemis-test3.artemis.cit.tum.de September 6, 2024 19:26 — with GitHub Actions Inactive
SimonEntholzer
SimonEntholzer previously approved these changes Sep 6, 2024
Copy link
Contributor

@SimonEntholzer SimonEntholzer left a comment

Choose a reason for hiding this comment

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

👍 fixes the not showing translation fields. Tested on Ts3

Copy link
Contributor

@pzdr7 pzdr7 left a comment

Choose a reason for hiding this comment

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

I left a suggestion regarding the spacing in the "Participation Mode" column. Adding another span into the div with justify-content-between has created some space that I think we could remove.


Can you also have a look at the failing client tests?

coderabbitai[bot]
coderabbitai bot previously approved these changes Sep 8, 2024
Co-authored-by: Patrik Zander <38403547+pzdr7@users.noreply.github.com>
JohannesWt pushed a commit that referenced this pull request Sep 23, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
Archived in project
Development

Successfully merging this pull request may close these issues.

6 participants