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

Fixed 24990: link doesn't redirect to dashboard #26100

Conversation

Usik2203
Copy link
Contributor

Description (*)

This PR fixed #24990 issue
1

Fixed Issues (if relevant)

  1. Admin Panel logo link is not directing to admin dashboard page #24990: Admin Panel logo link is not directing to admin dashboard page

Manual testing scenarios (*)

  1. ...
  2. ...

Questions or comments

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)
  • All automated tests passed successfully (all builds are green)

@Usik2203 Usik2203 requested a review from buskamuza as a code owner December 18, 2019 15:27
@magento-engcom-team magento-engcom-team added Partner: Atwix Pull Request is created by partner Atwix partners-contribution Pull Request is created by Magento Partner labels Dec 18, 2019
@m2-assistant
Copy link

m2-assistant bot commented Dec 18, 2019

Hi @Usik2203. Thank you for your contribution
Here is some useful tips 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

For more details, please, review the Magento Contributor Guide documentation.

@Usik2203
Copy link
Contributor Author

@magento give me test instance

@magento-engcom-team
Copy link
Contributor

Hi @Usik2203. Thank you for your request. I'm working on Magento instance for you

@magento-engcom-team
Copy link
Contributor

Hi @Usik2203, here is your new Magento instance.
Admin access: https://pr-26100.instances.magento-community.engineering/admin
Login: admin Password: 123123q

/**
* @param NavModel $navigation
* @param Status $status
*/
public function __construct(NavModel $navigation, Status $status)
public function __construct(NavModel $navigation, Status $status, ObjectManagerProvider $objectManagerProvider)
Copy link
Contributor

Choose a reason for hiding this comment

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

Why do you use ObjectManagerProvider instead of injecting just UrlInterface?

Copy link
Contributor Author

@Usik2203 Usik2203 Jan 8, 2020

Choose a reason for hiding this comment

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

I am getting such error "An error occurred during execution; please try again later" and status 500 while injecting UrlInterface directly in constructor of controller

@ghost ghost assigned lbajsarowicz Dec 18, 2019
@Usik2203 Usik2203 closed this Jan 14, 2020
@m2-assistant
Copy link

m2-assistant bot commented Jan 14, 2020

Hi @Usik2203, thank you for your contribution!
Please, complete Contribution Survey, it will take less than a minute.
Your feedback will help us to improve contribution process.

@Usik2203 Usik2203 reopened this Jan 14, 2020
@ghost ghost unassigned lbajsarowicz Jan 14, 2020
@Usik2203
Copy link
Contributor Author

@magento give me test instance

@magento-engcom-team
Copy link
Contributor

Hi @Usik2203. Thank you for your request. I'm working on Magento instance for you

@magento-engcom-team
Copy link
Contributor

Hi @Usik2203, here is your new Magento instance.
Admin access: https://pr-26100.instances.magento-community.engineering/admin_fbea
Login: a8bacc76 Password: 98beb8e12485
Instance will be terminated in up to 3 hours.

Copy link
Contributor

@lbajsarowicz lbajsarowicz left a comment

Choose a reason for hiding this comment

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

  1. Instantiation of the classes with new keyword
  2. Use of Object Manager.

@@ -57,7 +59,7 @@ public function __construct(NavModel $navigation, Status $status, ObjectManagerP
*/
public function indexAction()
{
$json = new JsonModel;
$json = new JsonModel();
Copy link
Contributor

Choose a reason for hiding this comment

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

Why are you instantiating classes with new instead of using Factory or just Dependency Injection?

/** @var \Magento\Backend\Model\UrlInterface $backendUrl */
$backendUrl = $this->objectManagerProvider->get()->get(\Magento\Backend\Model\UrlInterface::class);
/** @var UrlInterface $backendUrl */
$backendUrl = $this->objectManagerProvider->get()->get(UrlInterface::class);
Copy link
Contributor

Choose a reason for hiding this comment

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

You should inject that class to the constructor.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Hi @lbajsarowicz ,
Unfortunately I can't inject \Magento\Backend\Model\UrlInterfacedirectly to constructor. I get such error.
Screenshot 2020-02-03 at 18 24 27

Also I want to notice that construction like this $this->objectManagerProvider->get() is used in a lot of cases.

If such approach is wrong, maybe You could advise me something ?

Thanks a lot!

@@ -47,7 +49,7 @@ public function __construct(NavModel $navigation, Status $status, ObjectManagerP
$this->navigation = $navigation;
$this->status = $status;
$this->objectManagerProvider = $objectManagerProvider;
$this->view = new ViewModel;
$this->view = new ViewModel();
Copy link
Contributor

Choose a reason for hiding this comment

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

Why are you instantiating classes with new instead of using Factory or just Dependency Injection?

Copy link
Contributor Author

@Usik2203 Usik2203 Feb 3, 2020

Choose a reason for hiding this comment

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

I cannot create through Factory or Dependency Injection as usual I am getting such error
Screenshot 2020-02-18 at 10 28 44
Also I see a lot of cases in setup/src/Magento/Setup/Controller were these object was created with new

@ghost ghost assigned lbajsarowicz Jan 30, 2020
@Usik2203 Usik2203 requested a review from eduard13 February 18, 2020 16:39
@Usik2203
Copy link
Contributor Author

Hi @lbajsarowicz .
Do You have any news about this PR?
Thanks

Copy link
Contributor

@eduard13 eduard13 left a comment

Choose a reason for hiding this comment

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

Hi @Usik2203, could you please fix the conflicting file?
Thank you.

@ghost ghost assigned eduard13 Mar 27, 2020
@lbajsarowicz lbajsarowicz removed their assignment Apr 1, 2020
@lbajsarowicz lbajsarowicz requested review from lbajsarowicz and removed request for lbajsarowicz April 1, 2020 08:30
@lbajsarowicz
Copy link
Contributor

Sorry, I've tried to unassign me from the PR.

@engcom-Echo engcom-Echo self-assigned this Apr 1, 2020
@engcom-Echo engcom-Echo requested review from lbajsarowicz and removed request for lbajsarowicz April 1, 2020 12:14
@magento-engcom-team
Copy link
Contributor

Hi @lbajsarowicz, thank you for the review.
ENGCOM-7237 has been created to process this Pull Request

@m2-assistant
Copy link

m2-assistant bot commented Apr 2, 2020

Hi @Usik2203, thank you for your contribution!
Please, complete Contribution Survey, it will take less than a minute.
Your feedback will help us to improve contribution process.

@sdzhepa sdzhepa mentioned this pull request May 9, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Auto-Tests: Covered All changes in Pull Request is covered by auto-tests Award: bug fix Award: MFTF test coverage Component: Backend Component: Setup Partner: Atwix Pull Request is created by partner Atwix partners-contribution Pull Request is created by Magento Partner Progress: accept Release Line: 2.4
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Admin Panel logo link is not directing to admin dashboard page
6 participants