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

[NU-1719] Component statistics without restmodel #6258

Merged
merged 10 commits into from
Jul 4, 2024

Conversation

ForrestFairy
Copy link
Contributor

@ForrestFairy ForrestFairy commented Jun 24, 2024

Describe your changes

Remove creating ComponentListElement DTOs just to get component usages.
Instead, map of usages per DesignerWideComponentId is provided by ComponentService and then mapped by id to available components

Checklist before merge

  • Related issue ID is placed at the beginning of PR title in [brackets] (can be GH issue or Nu Jira issue)
  • Code is cleaned from temporary changes and commented out lines
  • Parts of the code that are not easy to understand are documented in the code
  • Changes are covered by automated tests
  • Showcase in dev-application.conf added to demonstrate the feature
  • Documentation added or updated
  • Added entry in Changelog.md describing the change from the perspective of a public distribution user
  • Added MigrationGuide.md entry in the appropriate subcategory if introducing a breaking change
  • Verify that PR will be squashed during merge

@ForrestFairy ForrestFairy changed the title Component statistics without restmodel [NU-1719] Component statistics without restmodel Jun 25, 2024
@ForrestFairy ForrestFairy force-pushed the no-restmodel-in-statistics branch from 3c50979 to b7ee445 Compare June 25, 2024 11:18
@ForrestFairy ForrestFairy marked this pull request as ready for review June 25, 2024 13:22
Copy link
Contributor

@lukasz-bigorajski lukasz-bigorajski left a comment

Choose a reason for hiding this comment

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

About counting components stats I will write later

@lukasz-bigorajski
Copy link
Contributor

Having 2 logics for counting usages can be misleading and error prone so we should use logic that we have currently. I am looking at componentsService now and I wonder if it will be enough to add method:

def getComponentsUsages(implicit user: LoggedUser): Future[Map[DesignerWideComponentId, Long]]

and then:

  override def getComponentsUsages(implicit user: LoggedUser): Future[Map[DesignerWideComponentId, Long]] =
    getUserAccessibleComponentUsages

but I have to confirm how is matched DesignerWideComponentId and ComponentId

Copy link
Contributor

github-actions bot commented Jul 1, 2024

created: #6318
⚠️ Be careful! Snapshot changes are not necessarily the cause of the error. Check the logs.

@ForrestFairy ForrestFairy force-pushed the no-restmodel-in-statistics branch from 5cfc2a3 to 70662d4 Compare July 4, 2024 06:30
Copy link
Contributor

@lukasz-bigorajski lukasz-bigorajski left a comment

Choose a reason for hiding this comment

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

LGTM

Have you manually tested this components usage count on local nu?

@ForrestFairy ForrestFairy force-pushed the no-restmodel-in-statistics branch from 7167f10 to adfd72a Compare July 4, 2024 11:02
@ForrestFairy ForrestFairy merged commit 94fdc7a into staging Jul 4, 2024
16 of 17 checks passed
@ForrestFairy ForrestFairy deleted the no-restmodel-in-statistics branch July 4, 2024 13:15
lukasz-bigorajski pushed a commit that referenced this pull request Jul 5, 2024
* Statistics for components look the same as while using restmodel, need to rewrite some tests due to the change

* In component usage statistic not all custom components were labeled as such

* Component usage is calculated by using one of componentService methods
And extracted some values as consts

* Replaced checking head with the whole list of components if it's from nussknacker package

* Added example dummy custom component to check if mechanism works properly
If componentId not found - it's connected to fragments, so we don't send that data

* Fix for 2.12 it couldn't compile 'removed' for map

---------

Co-authored-by: Szymon Bogusz <[email protected]>
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.

2 participants