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

EW-1071 Common Cartridge microservice creates boards during import #5435

Open
wants to merge 28 commits into
base: main
Choose a base branch
from

Conversation

psachmann
Copy link
Contributor

@psachmann psachmann commented Jan 16, 2025

Description

Common Cartridge microservice can now add boards with titles during import to a course. Additionally, the boards-client module was updated and moved to infra. OpenAPI client generation was aligned with the new way and npm script was added for easy use.

Links to Tickets or other pull requests

Approval for review

  • DEV: If api was changed - generate-client:server was executed in vue frontend and changes were tested and put in a PR with the same branch name.
  • QA: In addition to review, the code has been manually tested (if manual testing is possible)
  • All points were discussed with the ticket creator, support-team or product owner. The code upholds all quality guidelines from the PR-template.

@psachmann psachmann marked this pull request as ready for review January 21, 2025 12:25
@psachmann psachmann changed the title EW-1071 EW-1071 Common Cartridge microservice creates boards during import Jan 21, 2025
@Fshmit Fshmit self-requested a review January 22, 2025 08:38
@Fshmit
Copy link
Contributor

Fshmit commented Jan 22, 2025

@psachmann Implementation works well ✅.
Using by openApi generated objects or creating new DTOs to map the data into, will be discussed in the Arch chapter meeting by @alweber-cap on 23.01.2025. Depending on the decision, maybe we should create DTOs and mappers instead of using the auto-generated object directly in the implementation.

@@ -235,4 +235,24 @@ describe('CourseService', () => {
});
});
});

describe('create', () => {
Copy link
Contributor

Choose a reason for hiding this comment

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

The test exists already above.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Done.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants