Remove Magento_ReleaseNotification module and cleanup all traces of it #36849

hostep · 2023-02-11T11:15:44Z

Description (*)

The release notification module has been broken since Magento 2.4.0 as explained over here: #29363

Also after some more investigation, it still tries to fetch content from a remote endpoint on the magento.com domain that no longer seems to exist at the time of writing, for example: https://magento.com/release_notifications/2.2.4/community/en_us.json (I picked 2.2.4 because I know for sure that that release contained a release notification popup, as demonstrated in the second screenshot in #29363 (comment))

So to me it sounds like this module won't be used anymore now and in the future.

Since this module was referenced in the Magento_AdminAnalytics module, I also removed all traces over there and had to rework a little bit of the css there to make the line height in the popup match with how it was before (since it referenced some css from the release notification module).

Related Pull Requests

https://github.com/magento-gl/magento2-infrastructure/pull/16

Fixed Issues (if relevant)

Fixes Magento 2.4.0 release notification not showing up in backend #29363

Manual testing scenarios (*)

Since the removed module got referenced in the Magento_AdminAnalytics module, make sure that module still works properly
Easiest way to test is to temporarily modify the method Magento\AdminAnalytics\Model\Condition\CanViewNotification::isVisible and hardcode a return true; in its first line
After doing so, visit the dashboard in the backoffice and you should see a popup showing up, verify that it looks exactly like it looked before the removal of the ReleaseNotification module

Questions or comments

Is this PR considered backwards compatible?
Or should we instead of completely removing the module, just hardcode it to never show up again in the backoffice, so we don't get that weird half-rendered popup anymore you see when visiting the backoffice for the first time on every single new release of Magento like shown in the first screenshot of #29363 (comment) ?

Contribution checklist (*)

Pull request has a meaningful description of its purpose
All commits are accompanied by meaningful commit messages
All new or changed code is covered with unit/integration tests (if applicable)
README.md files for modified modules are updated and included in the pull request if any README.md predefined sections require an update
All automated tests passed successfully (all builds are green)

m2-assistant · 2023-02-11T11:15:47Z

Hi @hostep. Thank you for your contribution!
Here are some useful tips on how you can test your changes using Magento test environment.

Add the comment under your pull request to deploy test or vanilla Magento instance:

@magento give me test instance - deploy test instance based on PR changes
@magento give me 2.4-develop instance - deploy vanilla Magento instance

❗ Automated tests can be triggered manually with an appropriate comment:

@magento run all tests - run or re-run all required tests against the PR changes
@magento run <test-build(s)> - run or re-run specific test build(s)
For example: @magento run Unit Tests

<test-build(s)> is a comma-separated list of build names.

Allowed build names are:

Database Compare
Functional Tests CE
Functional Tests EE
Functional Tests B2B
Integration Tests
Magento Health Index
Sample Data Tests CE
Sample Data Tests EE
Sample Data Tests B2B
Static Tests
Unit Tests
WebAPI Tests
Semantic Version Checker

You can find more information about the builds here
ℹ️ Run only required test builds during development. Run all test builds before sending your pull request for review.

For more details, review the Code Contributions documentation.
Join Magento Community Engineering Slack and ask your questions in #github channel.

hostep · 2023-02-11T11:16:11Z

@magento run all tests

magento-automated-testing · 2023-02-11T11:16:20Z

The requested builds are added to the queue. You should be able to see them here within a few minutes. Please re-request them if they don't show in a reasonable amount of time.

hostep · 2023-02-11T12:26:56Z

The following static test failure will be resolved once magento/magento-coding-standard#432 is finally approved and released:

FILE: /var/www/html/app/design/adminhtml/Magento/backend/Magento_AdminAnalytics/web/css/source/_module.less
-----------------------------------------------------------------------------------------------------------
FOUND 0 ERRORS AND 1 WARNING AFFECTING 1 LINE
-----------------------------------------------------------------------------------------------------------
 46 | WARNING | CSS class names should be separated with "-" (dash) instead of "_" (underscore)
-----------------------------------------------------------------------------------------------------------

I'm currently not going to try to fix other test failures, until it's clear if this PR will be approved as is, or if it needs to be reworked completely.

Den4ik

@hostep I'm totally agree with you and also suggest to remove this module 👍
PR looks good for me, but let's try to check it more detailed

magento-automated-testing · 2023-03-07T10:06:29Z

Pull Requests are not mergeable to the mainline. Please merge the latest mainlines to your Pull Requests and restart the builds.

fredden

The changes here look good to me. The automation is saying that there are problems preventing this from being merged / tested.

hostep · 2023-03-09T06:53:25Z

I know, and I'm going to try to put some time into this whenever I get a final confirmation from Adobe that the PR will be accepted as is (because I have the feeling Adobe might not want to remove an entire module, and might want to just remove parts of the module...)

hostep · 2023-03-09T16:50:13Z

Rebased changes on latest 2.4-develop branch to solve the merge conflict with the composer.lock file (which will probably happen again in the future)

Got an agreement from the PO through @sidolov on Slack that the PR can be processed as-is and the entire module can be removed.
Internal devs will pick up the failing tests.

@magento run all tests

sidolov · 2023-03-09T17:11:05Z

@magento run all tests

magento-automated-testing · 2023-03-09T17:11:13Z

The requested builds are added to the queue. You should be able to see them here within a few minutes. Please re-request them if they don't show in a reasonable amount of time.

engcom-November · 2023-03-10T09:19:54Z

✔️ QA Passed
Preconditions: - Latest Magento 2.4-develop and PHP 8.1
Manual testing scenario:

Modified the method Magento\AdminAnalytics\Model\Condition\CanViewNotification::isVisible and hardcode a return true; in its first line
Visit Admin Dashboard and verify below text showing up for the 1st time login

Before: ✖️
Below text is displayed in the footer section of Dashboard page with invalid non clickable "Next" link before applying PR changes.

After: ✔️
No weird text / popup showing on the Dashboard page and footer section of Dashboard page with PR changes.

Builds are failed hence, moving this to extended testing.

magento-automated-testing · 2023-04-05T19:19:37Z

The requested builds are added to the queue. You should be able to see them here within a few minutes. Please re-request them if they don't show in a reasonable amount of time.

hostep · 2023-04-13T19:15:40Z

Rebased on latest 2.4-develop branch to fix merge conflicts caused by f87634a#diff-671e339cfcad7f363cb9b85f3a31ad8d749246162f93a2ade2b746dc1f4b72e7

@magento run all tests

Tests will still fail, and that's expected.

magento-automated-testing · 2023-04-13T19:15:51Z

The requested builds are added to the queue. You should be able to see them here within a few minutes. Please re-request them if they don't show in a reasonable amount of time.

Den4ik

@hostep Could you please verify static tests. They are related to changes

fredden · 2023-04-17T22:27:52Z

@Den4ik there are two errors that I can see in the Static Tests.

One is because Adobe Commerce ('B2B' & 'EE') still lists this module in its composer.json file. This is not something that we are able to fix and will need to be done by someone on Adobe staff (with access to the relevant repositories).

The other error is fixed by magento/magento-coding-standard#432 which has been merged in, but no release has yet been created containing this fix.

hostep · 2023-04-18T19:31:09Z

@Den4ik: indeed, like Dan says, the test failures are expected. Please read through all comments if you want to know the whole story.
Could you please remove the "needs update" label again if possible? Thanks!

engcom-Charlie · 2023-04-25T07:02:38Z

It seems that bot have moved this PR to Ready for testing from Pending Approval hence moving it back.

Thank you!

engcom-Charlie · 2023-09-06T13:58:38Z

We have got the approval for the false positive static test failure internal approval JIRA.

Still waiting for the other 2 mentioned in comment.

hostep · 2023-10-12T18:48:35Z

Rebased on latest 2.4-develop and fixed conflict in composer.lock file (which will most likely return if this PR stays open for another few months).

Coding standards 32 got released some weeks ago, which should already fix one failure in the static tests that no longer will trigger now. The others are still pending approval, maybe within 2 years we might be able to proceed here? Who knows ...

@magento run all tests

hostep · 2023-10-12T18:50:02Z

@magento run all tests

magento-automated-testing · 2023-10-12T18:50:13Z

The requested builds are added to the queue. You should be able to see them here within a few minutes. Please message the #magento-devops slack channel if they don't show in a reasonable amount of time and a representative will look into any issues.

ihor-sviziev · 2023-10-17T16:08:04Z

@engcom-Charlie, if I got right, we got the approval from PO, so we should change the status of this PR.
What is the status of all the linked internal issues/pull requests?

engcom-Charlie · 2023-10-18T06:23:26Z

Hi @ihor-sviziev,

As I have mentioned above, we have got the approval on internal JIRA of "Static test failure".

On the rest 2 apporval JIRA's which are mentioned here we are still waiting for all the required approvals there. Hence We will be keeping this PR in the same status for now.

Thank you!

ihor-sviziev · 2023-10-30T15:53:20Z

@hostep it feels like in Magento the module was not removed, but instead - marked as deprecated
3ee3962

hostep · 2023-10-30T16:00:43Z

@ihor-sviziev, yeah indeed, I was just looking into the same changes 😄

I think that this won't fix #29363, but would need to test it out first (maybe later this week).

And no idea if this PR is now considered obsolete, or if one day we can continue with it (maybe for Magento 2.4.8 ...)

Some feedback from product owner would be appreciated 🙂

engcom-Charlie · 2023-11-20T13:33:41Z

As informed by Product Owner, Adobe Magento internal team will be taking care of this feature in future releases.

Magento will be first deprecating the modules and then will be removing it. Hence closing the PR now.

Thank you!

hostep · 2023-11-20T13:37:57Z

Thanks for the info @engcom-Charlie !

Can you make sure internal team is aware about dependency of the AdminAnalytics module on the ReleaseNotification module, so if they remove this module, they will need to fix AdminAnalytics module (not only in PHP, but also in javascript & css area's). Like I proposed in this PR.

m2-community-project bot added Priority: P2 A defect with this priority could have functionality issues which are not to expectations. Progress: pending review labels Feb 11, 2023

Den4ik reviewed Feb 12, 2023

View reviewed changes

m2-community-project bot assigned Den4ik Feb 12, 2023

m2-community-project bot added Progress: review and removed Progress: pending review labels Feb 12, 2023

fredden approved these changes Mar 7, 2023

View reviewed changes

m2-community-project bot assigned fredden Mar 7, 2023

m2-community-project bot added Progress: ready for testing and removed Progress: review labels Mar 7, 2023

engcom-November added the Triage: Dev.Experience Issue related to Developer Experience and needs help with Triage to Confirm or Reject it label Mar 8, 2023

Den4ik requested review from fredden and Den4ik March 8, 2023 21:15

fredden approved these changes Mar 8, 2023

View reviewed changes

hostep force-pushed the fix-for-issue-29363 branch from bd1f19d to 21bb0ff Compare March 9, 2023 16:48

Den4ik approved these changes Mar 9, 2023

View reviewed changes

engcom-November self-assigned this Mar 10, 2023

engcom-November added the Progress: testing in progress label Mar 10, 2023

m2-community-project bot removed the Progress: ready for testing label Mar 10, 2023

engcom-November added the Progress: extended testing label Mar 10, 2023

hostep force-pushed the fix-for-issue-29363 branch from 2570bd8 to d228d71 Compare April 13, 2023 19:14

Den4ik requested changes Apr 17, 2023

View reviewed changes

m2-community-project bot added Progress: needs update and removed Progress: pending approval labels Apr 17, 2023

Den4ik approved these changes Apr 18, 2023

View reviewed changes

m2-community-project bot added Progress: ready for testing and removed Progress: needs update labels Apr 18, 2023

sinhaparul added the Project: Community Picked PRs upvoted by the community label Apr 24, 2023

m2-community-project bot added Progress: pending approval and removed Progress: ready for testing labels Apr 25, 2023

Remove Magento_ReleaseNotification module and cleanup all traces of it.

f7e9f87

hostep force-pushed the fix-for-issue-29363 branch from d228d71 to f7e9f87 Compare October 12, 2023 18:46

engcom-Charlie closed this Nov 20, 2023

m2-community-project bot removed the Progress: pending approval label Nov 20, 2023

Remove Magento_ReleaseNotification module and cleanup all traces of it #36849

Remove Magento_ReleaseNotification module and cleanup all traces of it #36849

Uh oh!

Conversation

hostep commented Feb 11, 2023 • edited by engcom-Charlie Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Description (*)

Related Pull Requests

Fixed Issues (if relevant)

Manual testing scenarios (*)

Questions or comments

Contribution checklist (*)

Uh oh!

m2-assistant bot commented Feb 11, 2023

Uh oh!

hostep commented Feb 11, 2023

Uh oh!

magento-automated-testing bot commented Feb 11, 2023

Uh oh!

hostep commented Feb 11, 2023

Uh oh!

Den4ik left a comment

Choose a reason for hiding this comment

Uh oh!

magento-automated-testing bot commented Mar 7, 2023

Uh oh!

fredden left a comment

Choose a reason for hiding this comment

Uh oh!

hostep commented Mar 9, 2023

Uh oh!

hostep commented Mar 9, 2023

Uh oh!

sidolov commented Mar 9, 2023

Uh oh!

magento-automated-testing bot commented Mar 9, 2023

Uh oh!

engcom-November commented Mar 10, 2023 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

magento-automated-testing bot commented Apr 5, 2023

Uh oh!

hostep commented Apr 13, 2023 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

magento-automated-testing bot commented Apr 13, 2023

Uh oh!

Den4ik left a comment

Choose a reason for hiding this comment

Uh oh!

fredden commented Apr 17, 2023

Uh oh!

hostep commented Apr 18, 2023

Uh oh!

engcom-Charlie commented Apr 25, 2023

Uh oh!

engcom-Charlie commented Sep 6, 2023

Uh oh!

hostep commented Oct 12, 2023

Uh oh!

hostep commented Oct 12, 2023

Uh oh!

magento-automated-testing bot commented Oct 12, 2023

Uh oh!

ihor-sviziev commented Oct 17, 2023

Uh oh!

engcom-Charlie commented Oct 18, 2023

Uh oh!

ihor-sviziev commented Oct 30, 2023

Uh oh!

hostep commented Oct 30, 2023 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

engcom-Charlie commented Nov 20, 2023

Uh oh!

hostep commented Nov 20, 2023 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

Uh oh!

hostep commented Feb 11, 2023 •

edited by engcom-Charlie

Loading

engcom-November commented Mar 10, 2023 •

edited

Loading

hostep commented Apr 13, 2023 •

edited

Loading

hostep commented Oct 30, 2023 •

edited

Loading

hostep commented Nov 20, 2023 •

edited

Loading