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

Fix for error while archiving a product #2650

Merged
merged 3 commits into from
Aug 13, 2017
Merged

Conversation

foladipo
Copy link
Contributor

@foladipo foladipo commented Aug 8, 2017

This PR resolves #2620. The said issue was a bug report for an error that occurred whenever a user tried to archive a product. These trials consequently failed. And it was all due to the wrong use of a certain function's parameter.

Testing steps

  1. Login as admin.
  2. Click once on the default product (i.e Basic Reaction Product) or another product (if you've added any).
  3. Select "Clone product" in the side menu.
  4. Double-click the newly created product to open it (scroll the page to find it if necessary).
  5. Click on "Archive" in the top menu.
  6. The archiving operation should now be completed without any errors. Similarly, check the terminal/command prompt and the error reported in #2620 should be absent.

@foladipo foladipo changed the base branch from master to development August 8, 2017 13:30
@foladipo foladipo changed the base branch from development to marketplace August 8, 2017 13:55
@brent-hoover
Copy link
Collaborator

@foladipo a couple of notes

  1. Generally we want the PR title to be self-explanatory so whenever possible make it "Fix for error when archiving product" rather than just "Fix for issue".
  2. You don't need to include things like "open your terminal prompt" in testing instructions. Basically your testing instructions could start with step 4. There are certainly worse things than being over-explicit but probably better to keep them shorter.
  3. If this PR is ready you will want to assign a reviewer (for now, me) and also assign the PR to me. I will get a notification that you have requested a review and I can take a look at it.

@foladipo foladipo changed the title [WIP] Bug fix for #2620 [WIP] Fix for error while archiving a product Aug 9, 2017
@foladipo
Copy link
Contributor Author

foladipo commented Aug 9, 2017

Thanks for your feedback @zenweasel . I've resolved the first two points you raised. I will add you as a reviewer (and thus make you get a notification) as soon as I finish some checks today.

@foladipo foladipo changed the title [WIP] Fix for error while archiving a product Fix for error while archiving a product Aug 9, 2017
@foladipo foladipo requested a review from brent-hoover August 9, 2017 20:15
Copy link
Collaborator

@brent-hoover brent-hoover left a comment

Choose a reason for hiding this comment

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

Tested, verified fixed

@brent-hoover
Copy link
Collaborator

@foladipo Not sure how this is happening since you didn't touch these files but we have 3 files that are failing ESLint. Let's fix those now while we catch them.

@brent-hoover brent-hoover merged commit 7001245 into marketplace Aug 13, 2017
@brent-hoover brent-hoover deleted the folusho-bug-fix-2620 branch August 13, 2017 23:13
@spencern spencern mentioned this pull request Oct 11, 2017
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