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 adobe data-collection code and modal #43

Closed
Vinai opened this issue Oct 7, 2023 · 6 comments · Fixed by #45, #49, #50 or #51
Closed

Remove adobe data-collection code and modal #43

Vinai opened this issue Oct 7, 2023 · 6 comments · Fixed by #45, #49, #50 or #51

Comments

@Vinai
Copy link
Contributor

Vinai commented Oct 7, 2023

Both the adobe data-collection modal and the code to collect the data should be removed.

Preconditions and environment

  • Mage-OS 1.0.0 preview

Steps to reproduce

Install the 1.0.0 preview and log into the backend as an admin user.

Expected result

I see the admin dashboard.

Actual result

I see the data-collection agreement modal dialog.

Bildschirmfoto 2023-10-08 um 08 02 43
@Vinai Vinai changed the title Remove data-collection code and modal Remove adobe data-collection code and modal Oct 7, 2023
@rhoerr
Copy link
Contributor

rhoerr commented Oct 7, 2023

Are we looking to remove those specific aspects, or the entire AdminAnalytics module? I believe that's the entire purpose of the module. https://github.com/mage-os/mageos-magento2/tree/2.4-develop/app/code/Magento/AdminAnalytics

The package code is magento/module-admin-analytics

@Vinai
Copy link
Contributor Author

Vinai commented Oct 7, 2023

Removing it would be best, however, because of upstream compatibility we maybe could keep it but short-circuit all functionality?

@Vinai
Copy link
Contributor Author

Vinai commented Oct 7, 2023

Or, maybe we remove it after all, since resolving merge conflicts with upstream will probably be rather simple :)

EDIT: more info and discussion in the PR #45 from @pykettk

@Vinai
Copy link
Contributor Author

Vinai commented Oct 9, 2023

Reopening because the tracking URL still needs to be removed.

Vinai added a commit to Vinai/mageos-magento2 that referenced this issue Oct 9, 2023
This change ensures that in case the tracking-feature is enabled
in the admin, Mage-OS will not be tracked as Magento Open Source
or Enterprise Edition installations incorrectly.

In order to use admin analytics, a custom tracking_url script needs to
be configured on the tracking block.

Fixes mage-os#43
mage-os-ci pushed a commit that referenced this issue Oct 9, 2023
This change ensures that in case the admin tracking-feature is enabled
in the config, Mage-OS will not be tracked as Magento Open Source
or Enterprise Edition installations incorrectly.

In order to use admin analytics, a custom tracking_url script needs to
be configured on the tracking block.

Fixes #43
@Vinai Vinai reopened this Oct 9, 2023
@Vinai
Copy link
Contributor Author

Vinai commented Oct 9, 2023

The modal is still showing upon the first login.

It is a ui component, so making the block depend on the configuration flag didn't do the trick.
In app/code/Magento/AdminAnalytics/view/adminhtml/layout/adminhtml_dashboard_index.xml:

<uiComponent name="admin_usage_notification" ifconfig="admin/usage/enabled">

may do the trick.

Vinai added a commit to Vinai/mageos-magento2 that referenced this issue Oct 9, 2023
Previously, the admin analytics modal was displayed, even if the feature
was disabled in the system configuration.

Fixes mage-os#43
@Vinai Vinai closed this as completed in #50 Oct 9, 2023
Vinai added a commit that referenced this issue Oct 9, 2023
* Hide admin-analytics modal if config is disabled

Previously, the admin analytics modal was displayed, even if the feature
was disabled in the system configuration.

Fixes #43
@Vinai Vinai reopened this Oct 9, 2023
@Vinai
Copy link
Contributor Author

Vinai commented Oct 9, 2023

The modal still displays, because the default config value is evaluated as " 0 ", which is true when used with isSetFlag. Without wrapping space it evaluates correctly to false.

Vinai added a commit to Vinai/mageos-magento2 that referenced this issue Oct 9, 2023
Previously the value was evaluated as " 0 ", which is true when used
with isSetFlag. Without wrapping space it evaluates correctly to false.

Closes mage-os#43
@Vinai Vinai closed this as completed in #51 Oct 9, 2023
Vinai added a commit that referenced this issue Oct 9, 2023
Previously the value was evaluated as " 0 ", which is true when used
with isSetFlag. Without wrapping space it evaluates correctly to false.

Closes #43
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment