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

BC-6870 - Create unified board persistence entity #4919

Closed
wants to merge 27 commits into from

Conversation

uidp
Copy link
Contributor

@uidp uidp commented Apr 11, 2024

Description

To make it easier to extend the board we want to implement a unified persistence entity for board nodes.

Currently we have implemented specific persistence entity classes for each board node type (Inheritance Mapping). That is not necessary because:

  • The board node data is stored in one mongodb collection regardless of the type of the node
  • We are mapping the persistence entities to specific domain objects
  • so the persistence entity can be basically seen a just a data container

The mapping domain objects ist currently implemented as a builder using the visitor pattern which requires specific mapping code for each board node type.

We should remove this unnecessary complexity.

Links to Tickets or other pull requests

BC-6870

Changes

Datasecurity

Deployment

New Repos, NPM pakages or vendor scripts

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.

Notice: Please remove the WIP label if the PR is ready to review, otherwise nobody will review it.

@uidp uidp added the WIP This feature branch is in progress, do not merge into master. label Apr 11, 2024
import { AnyBoardNode, AllBoardNodeProps } from '../../domain';

const PATH_SEPARATOR = ',';
Copy link
Contributor Author

Choose a reason for hiding this comment

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

Can be imported from src/modules/board/poc/domain/path-utils.ts

/* ExternalToolElement props */
// TODO migration and only store ids not the whole entity
@Property({ nullable: true, fieldName: 'contextExternalToolId' })
contextExternalToolId?: ObjectId;
Copy link
Contributor Author

Choose a reason for hiding this comment

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

I think we should use a "virtual property" like here like:

_contextExternalToolId?: ObjectId;
get contextExternalToolId(): EntityId | undefined {
    return this._contextExternalToolId.toHextString();
}
set contextExternalToolId(id?: EntityId) {
    this._contextExternalToolId = id ? new ObjectId(id) : undefined;
}

Maybe this could even be modeled later using a ContextTrait with contextType and contextId similar to ColumnBoard. So they could share the same trait.

Copy link

sonarcloud bot commented Apr 25, 2024

Quality Gate Failed Quality Gate failed

Failed conditions
76.5% Coverage on New Code (required ≥ 80%)
56 Uncovered Lines on New Code (required ≤ 0)

See analysis details on SonarCloud

@uidp uidp removed the WIP This feature branch is in progress, do not merge into master. label Apr 25, 2024
@uidp uidp closed this Apr 25, 2024
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.

4 participants