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

feat: Support publishing to Business Accelerator Hub (#61) #133

Draft
wants to merge 1 commit into
base: main
Choose a base branch
from

Conversation

nirooxx
Copy link
Contributor

@nirooxx nirooxx commented Feb 25, 2025

Implements package separation and visibility handling for ORD compliance with Business Accelerator Hub.

  • Splits APIs and Events into distinct packages
  • Ensures visibility-based package assignment (public vs. internal)
  • Implements dynamic package descriptions
  • Improves package filtering to include only assigned resources

* @returns {object} Object containing the description and short description.
*/
const generatePackageDescriptions = (name, type, isInternal) => {
const visibility = isInternal ? "internal" : "public";
Copy link
Member

@aramovic79 aramovic79 Feb 25, 2025

Choose a reason for hiding this comment

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

I think there is yet another visibility to be considered.

cc: @pavelkornev @Fannon @swennemers

Copy link
Contributor Author

Choose a reason for hiding this comment

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

If private is also needed, I’ll adjust the logic accordingly.

Comment on lines +76 to +85
createPackage(name, "-api:v1", "APIs"),
createPackage(name, "-api-internal:v1", "APIs", true),
createPackage(name, "-event:v1", "Events"),
createPackage(name, "-event-internal:v1", "Events", true),
createPackage(name, "-consumptionBundle:v1", "Consumption Bundles"),
createPackage(name, "-consumptionBundle-internal:v1", "Consumption Bundles", true),
createPackage(name, "-integrationDependency:v1", "Integration Dependencies"),
createPackage(name, "-integrationDependency-internal:v1", "Integration Dependencies", true),
createPackage(name, "-entityType:v1", "Entity Types"),
createPackage(name, "-entityType-internal:v1", "Entity Types", true),
Copy link
Member

Choose a reason for hiding this comment

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

@nirooxx: Not sure if it the right approach to "by default" generate packages for all 3 visibilities.
I think we should first check e.g. if there is at least one api/eventResource(or consumptionBundle..) with some visibility(public, internal, private) and then generate the corresponding package. I see no value of having an empty package, which will almost always happen if we leave the current implementation. Also, it doesn't make sense to me to have entityType with visibility public which is "referenced"(e.g. by entityTypeMappings[N].entityTypeTargets[N].ordId by some private apiResource.
@Fannon @pavelkornev @swennemers : Do you agree?

Copy link
Contributor

Choose a reason for hiding this comment

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

Please avoid empty packages.

For the reference, you should care about ghe visibility. The tool using ORD has to filter accordingly. It makes sense to me to have entityType with visibility public which is "referenced"(e.g. by entityTypeMappings[N].entityTypeTargets[N].ordId by some private apiResource.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

@aramovic79 : At the end of the process, _filterUnusedPackages() in ord.js removes any unused packages to ensure that only those referenced in API, Event, or Consumption Resources are included in the ORD document.

@aramovic79 aramovic79 marked this pull request as draft February 25, 2025 12:44
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