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

Remove widget export feature #19340

Conversation

maxiadlovskii
Copy link
Contributor

@maxiadlovskii maxiadlovskii commented May 14, 2024

Description

This PR

  • removes code related to the export widget feature
  • add the plug for the non-enterprise users with a promotion of the new feature
Screenshot 2024-05-15 at 09 33 58
  • do the groundwork for the enterprise plugin to be able to use this exporting feature there

Using our plugin system faces a couple of problems:

  • We already have actions from enterprise (fe switch to log view action) that we place into the dropdown menu and now we have to add them into the menu as well. We would like to keep only already existing views.widgets.actions. To be able to separate actions by position we extend WidgetActionType. Now it has a prop position where we can define where exactly we would like to render it. We also add new component props there. We use it instead of action and title to render the action component. So now we can use action and title OR component props to render
    action.

  • we have to keep exporting for the messages widget. We add export messages action component into our plugins views.widgets.actions in the core.

  • and we have to add the plug only for the non-enterprise users. To do that we provide ExportWidgetAction with position menu and ExportWidgetActionDelegate as a component. ExportWidgetActionDelegate looks at views.components.widgets.exportAction entity. If there is no component to render it renders a plug

Motivation and Context

fix: #19339

How Has This Been Tested?

Screenshots (if appropriate):

Types of changes

  • Bug fix (non-breaking change which fixes an issue)
  • New feature (non-breaking change which adds functionality)
  • Refactoring (non-breaking change)
  • Breaking change (fix or feature that would cause existing functionality to change)

Checklist:

  • My code follows the code style of this project.
  • My change requires a change to the documentation.
  • I have updated the documentation accordingly.
  • I have read the CONTRIBUTING document.
  • I have added tests to cover my changes.

/jenkins-pr-deps https://github.com/Graylog2/graylog-plugin-enterprise/pull/7273

@maxiadlovskii maxiadlovskii marked this pull request as ready for review May 15, 2024 08:45
Copy link
Contributor

@linuspahl linuspahl left a comment

Choose a reason for hiding this comment

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

Just added some suggestions related to the wording.

message="Migrate export widget feture to enterprise"

issues=["19339", "graylog-plugin-enterprise#7272"]
pulls=["19340", "graylog-plugin-enterprise#7273"]
Copy link
Contributor

Choose a reason for hiding this comment

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

There is a typo in feture.

Since this feature has never release, wouldn't it make sense to just move change log in #19140 to the enterprise repo and not add a change log in this PR?

Copy link
Contributor

Choose a reason for hiding this comment

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

This sounds like we are removing the message widget export feature from open source.

Copy link
Member

Choose a reason for hiding this comment

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

I think we should just remove the changelog snippet in core, because the change is not affecting anything that has been shipped before.

Copy link
Member

Choose a reason for hiding this comment

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

We should also remove issue-19139.toml.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

@dennisoelkers About issue-19139.toml we moved the messages export action to icon. Maybe it makes sense to keep at least that?

Copy link
Member

Choose a reason for hiding this comment

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

That makes sense, yes!

const Explanation = () => (
<span>Export Aggregation widget feature is available for the enterprise version.
Graylog provides option to export your data into most popular file formats such as
CSV, JSON, YAML, XML etc.
Copy link
Contributor

Choose a reason for hiding this comment

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

Shouldn't Aggregation be lowercase?
I think either provides an option or provides options sounds a bit better.

Copy link
Member

Choose a reason for hiding this comment

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

What about:

Exporting widget results is an enterprise-only feature. With an enterprise license, you can export your results as Excel (xlsx), CSV, JSON, YAML or XML.

Please [click here] for more details.

With some helpful link included.

Copy link
Member

@dennisoelkers dennisoelkers left a comment

Choose a reason for hiding this comment

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

Looks good to me, works fine!

@maxiadlovskii maxiadlovskii merged commit 34aecf5 into master May 16, 2024
6 checks passed
@maxiadlovskii maxiadlovskii deleted the feat/core-migrate-export-widget-feature-to-enterprise/issue-19339 branch May 16, 2024 15:16
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.

Migrate export widget feature from core to enterprise
3 participants