Skip to content

MQE-1427 - Support _CREDS in <magentoCLI> action and in Data #368

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

Merged
merged 42 commits into from
Jun 28, 2019

Conversation

ivy00johns
Copy link
Contributor

@ivy00johns ivy00johns commented Jun 13, 2019

Description

  • Adding support for _CREDS to the <magentoCLI> and <createData> actions.

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/verification tests (if applicable)
  • All automated tests passed successfully (all builds on Travis CI are green)
  • Changes to Framework doesn't have backward incompatible changes for tests or have related Pull Request with fixes to tests

- Adding new logic to support _CREDS in magentoCLI, createData and updateData.
- Removing "updateData", does NOT contain a <field>.
- Correcting the magentoCLI action when secret data is present.
- Adding support for createData when secret data is present.
@coveralls
Copy link

Coverage Status

Coverage decreased (-1.5%) to 54.441% when pulling 5fc6347 on MQE-1427 into 41caa75 on develop.

@coveralls
Copy link

coveralls commented Jun 13, 2019

Coverage Status

Coverage decreased (-0.1%) to 54.534% when pulling c5286e6 on MQE-1427 into 4d55d3f on develop.

@ivy00johns
Copy link
Contributor Author

Working on the Travis CI build issues now.

- Adding a missing annotation to a doc block.
- Adjusting line lengths.
- Replaced array with short array.
Copy link
Contributor

@KevinBKozan KevinBKozan left a comment

Choose a reason for hiding this comment

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

Overall idea and design is in the right direction, just had a few thoughts about simplifying some of the code structure!

Also, please add new verification tests to test the secret field implementation (I don't think we have a test dedicated to secret actions, would be a good place to start one).

Copy link
Contributor

@KevinBKozan KevinBKozan left a comment

Choose a reason for hiding this comment

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

Two small things you forgot to include in your commits I believe.

Also, please create a new verification test to test all of the available secret actions

@ivy00johns
Copy link
Contributor Author

Corrected Kevin's comments minus the Verification Test. Working on that now.

- Correcting ActionMergeUtil so it doesn't use fillSecretField when the creds aren't present.
- Correcting an issue in the verification test.
- Updating verification test so it is compatible with the latest version of the Test Generator.
- Correcting verification test issues.
- Correcting line lengths for the Travis CI build.
- Correcting Travis CI issues regarding multi line functions.
- Correcting multiline function errors in Travis CI.
- Updating the function signature for the CLI function to match the other CLI function casing.
- Adding new function to properly decrypt CLI actions.
- Adding MagentoCLISecret to the ActionMergeUtil.
- Correcting verification test issue due to casing.
- Correcting verification test issues.
Copy link
Contributor

@KevinBKozan KevinBKozan left a comment

Choose a reason for hiding this comment

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

2 Small changes, plus one change to PersistedObjectHandler.php:

In createEntity() you'll need to loop through them and use the new decryptAllSecretsInString() function.

- Correcting verification test issues around extra new lines. Removed the extra new lines.
- Correcting an issue around null values in the PersistedObjectHandler.
@ivy00johns
Copy link
Contributor Author

Changes submitted for the PersistedObjectHandler.

- Correcting verification tests by replacing null with [].
Copy link
Contributor

@KevinBKozan KevinBKozan left a comment

Choose a reason for hiding this comment

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

QA and CR complete, feature is good to go 👍

@KevinBKozan KevinBKozan merged commit 09f81b6 into develop Jun 28, 2019
@KevinBKozan KevinBKozan deleted the MQE-1427 branch July 26, 2019 19:18
magento-devops-reposync-svc pushed a commit that referenced this pull request Dec 11, 2023
ACQE-5871: Add ability to increase default MagentoCLI timeout
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.

3 participants