Skip to content

FINERACT-2481: Remove Pentaho reports from initial data#5501

Closed
Kenzy-Ragab wants to merge 2 commits intoapache:developfrom
Kenzy-Ragab:FINERACT-2481-remove-pentaho-initial-data
Closed

FINERACT-2481: Remove Pentaho reports from initial data#5501
Kenzy-Ragab wants to merge 2 commits intoapache:developfrom
Kenzy-Ragab:FINERACT-2481-remove-pentaho-initial-data

Conversation

@Kenzy-Ragab
Copy link

@Kenzy-Ragab Kenzy-Ragab commented Feb 15, 2026

Description

This PR removes 44 legacy Pentaho report entries from the initial system data to clean up the reporting metadata.

Changes Proposed

  • New Migration: Added 0210_remove_pentaho_legacy_data.xml.
  • Surgical Cleanup: Explicitly listed 44 reports in m_report_parameter and stretchy_report for targeted removal.
  • Permission Cleanup: Removed associated permissions in m_permission using pattern matching.
  • Integrity: Deletion order follows (Child -> Parent) to respect Foreign Key constraints.

Impact

  • Database: Cleaner initial state without obsolete metadata.
  • UI: Removed deprecated Pentaho reports and permissions from management screens.

Checklist

  • Migration ID follows sequential order (0210).
  • Deletion order respects Foreign Key constraints.
  • Explicit naming used for reports for maximum safety.

Copy link
Contributor

@IOhacker IOhacker left a comment

Choose a reason for hiding this comment

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

Hello, the PR is only about the data. Please remove the non required java class and don't include any sample_data sql file. #5491

The PR must be signed and the title must folllow the convention.

@Kenzy-Ragab
Copy link
Author

Hi @IOhacker, thank you for the feedback. I understand the requirement now. I will revert the Java changes and the sample data SQL files, keeping only the Liquibase migration for the data cleanup. I will also ensure the commit is signed and the PR title follows the convention. I'll push the updates shortly.

@IOhacker
Copy link
Contributor

IOhacker commented Feb 15, 2026 via email

@Kenzy-Ragab Kenzy-Ragab force-pushed the FINERACT-2481-remove-pentaho-initial-data branch 2 times, most recently from 33f5c22 to 8c3b97a Compare February 15, 2026 22:46
@Kenzy-Ragab
Copy link
Author

Hi @IOhacker, I have updated the PR according to your feedback.
Reverted Java changes and sample data SQL files (kept only Liquibase migration).
Updated the commit message to follow the convention.
Signed the commit with GPG (it should show as Verified now).
Ready for your review!

@IOhacker
Copy link
Contributor

@Kenzy-Ragab I suggest to update the PR description. The SQL I think that could be more explicit about the report names.

@Kenzy-Ragab
Copy link
Author

Hi @IOhacker,

Thank you for the guidance. I'm currently refining the SQL to be more explicit as requested. However, I have a quick architectural question before I push the final list:

Given that we have over 40+ Pentaho-related entries (between reports and permissions), listing them all explicitly in the IN clause will make the Liquibase script quite lengthy and harder to maintain if any similar legacy data is found later.

What do you think of this balanced approach?

  • Use the Pattern for permissions: LIKE 'READ_%_PENTAHO' (since they all follow this unique naming convention).
  • Use the Category for reports: report_category = 'Pentaho'.
  • I will add a detailed XML comment above the changeSet listing the major reports being removed as a reference for future developers.

This keeps the code clean and DRY while remaining explicit about the target group. Or would you prefer every single report name to be hardcoded in the script for maximum safety?

@IOhacker
Copy link
Contributor

@Kenzy-Ragab my request about explicit setting the name of the Pentaho report is because the DB could have another custom reports with PENTAHO in its name.

@Kenzy-Ragab
Copy link
Author

@IOhacker Got it! That makes total sense to avoid accidental deletion of custom reports. I have compiled the explicit list of the 44 core reports and updated the migration accordingly. Pushing the changes now.

@Kenzy-Ragab Kenzy-Ragab force-pushed the FINERACT-2481-remove-pentaho-initial-data branch from 8c3b97a to c7998d9 Compare February 17, 2026 04:16
@Kenzy-Ragab Kenzy-Ragab changed the title FINERACT-2481: Remove Pentaho reports from initial data and system FINERACT-2481 Remove Pentaho reports from the initial data used by the report table Feb 17, 2026
@Kenzy-Ragab Kenzy-Ragab changed the title FINERACT-2481 Remove Pentaho reports from the initial data used by the report table FINERACT-2481: Remove Pentaho reports from initial data Feb 17, 2026
@Kenzy-Ragab
Copy link
Author

Hi @IOhacker, I have updated the migration with the explicit list of 44 reports and handled the permissions as discussed. I see some CI failures; could you please check if these are related to existing tests depending on this legacy data, or if it's an environment issue? Thank you for your patience!

@Kenzy-Ragab
Copy link
Author

Hi @IOhacker, I've closed this PR to provide a fresh, cleaner version that consolidates all the discussed changes into a single, well-structured commit. I’ll link the new PR shortly. Thank you for the great feedback so far!

@adamsaghy
Copy link
Contributor

You can do that with a simple rebase

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