[TASK] Splitted Magento1 and Magento2 commands and added Crack_AdminP…#82
[TASK] Splitted Magento1 and Magento2 commands and added Crack_AdminP…#82lewisvoncken wants to merge 1 commit intoHypernode:masterfrom
Conversation
…assword Magento 2 Command
|
@vdloo can you please give feedback on this Pull Request? |
|
hi @lewisvoncken I'll take a good look next week, but the first thing I notice is that this basically duplicates a lot of code like from here. Isn't there a better way to do that? Could you also explain a bit in the description of this PR why this change is necessary and what problem it solves? Also please keep your commit titles under the 50 chars. |
| $this->userModel = $userModel; | ||
| $this->storeManager = $storeManager; | ||
| $this->encryptor = $encryptor; | ||
| $this->filesystem = $filesystem; |
| protected function generateSpecialWordlist() | ||
| { | ||
| $admins = $this->userModel->getCollection(); | ||
| $stores = $this->storeManager->getStores(); |
| */ | ||
| protected function getAdmins() | ||
| { | ||
| $admins = $this->userModel->getCollection(); |
| if ($engineType = $input->getOption('engine')) { | ||
| $factory->setEngineType($engineType); | ||
| } | ||
| $factory->setEncryptor($this->encryptor); |
| { | ||
| $path = realpath($this->specialPath); | ||
| if (file_exists($path) | ||
| && strpos($path, $this->filesystem->getDirectoryRead('tmp')->getAbsolutePath()) === 0) { |
| $generator->setStores($stores); | ||
| $words = $generator->generate(); | ||
|
|
||
| $directoryRead = $this->filesystem->getDirectoryRead('tmp'); |
|
@vdloo I have added comments according the Magento 2 Specific code. |
right, but I mean isn't there some nice higher level abstraction that can be made so the common parts aren't duplicated? |
AlexanderGrooff
left a comment
AlexanderGrooff
left a comment
There was a problem hiding this comment.
Your code seems to contain a lot of violations of the DRY principle. I would like to see this improved before I can approve this.
|
@vdloo Can you please apply changes as you recommended because the functionality is very useful? |
|
@AlexanderGrooff and @vdloo |
|
@lewisvoncken you asked for feedback on this PR and I mentioned before that in this PR basically a bunch of code is copy-pasted from here and slightly altered. I'm sure there's a nicer way to do that where only the relevant changes for m1 / m2 are abstracted out so that there is not this much duplicate code for everything else that is not different. I'm OK with merging this if you really need it but I don't think we should copy-paste code like this if there's just minor changes, this makes it hard to spot what exactly is different here from the other unaltered implementation. |
| $directoryRead = $this->filesystem->getDirectoryRead('tmp'); | ||
| $dir = $directoryRead->getAbsolutePath('password-cracker'); | ||
|
|
||
| $io = new \Varien_Io_File; |
There was a problem hiding this comment.
I don't think this class exists in M2, should be replaced with \Magento\Framework\Filesystem\Io\File; See: https://mage2.pro/t/topic/38
4 participants
…assword Magento 2 Command