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

Generate clear message when recipe exists during creation #702

Merged
merged 8 commits into from
May 12, 2021

Conversation

christianlupus
Copy link
Collaborator

Closes #690

@codecov
Copy link

codecov bot commented Apr 28, 2021

Codecov Report

Merging #702 (09661c9) into master (2fbdea8) will decrease coverage by 0.00%.
The diff coverage is 0.00%.

❗ Current head 09661c9 differs from pull request most recent head 6da7ae8. Consider uploading reports for the commit 6da7ae8 to get more accurate results
Impacted file tree graph

@@             Coverage Diff             @@
##             master    #702      +/-   ##
===========================================
- Coverage      0.95%   0.95%   -0.01%     
- Complexity      458     461       +3     
===========================================
  Files            18      19       +1     
  Lines          1460    1472      +12     
===========================================
  Hits             14      14              
- Misses         1446    1458      +12     
Flag Coverage Δ Complexity Δ
integration 0.00% <0.00%> (ø) 461.00 <1.00> (+3.00)
unittests 0.95% <0.00%> (-0.01%) 461.00 <1.00> (+3.00) ⬇️

Flags with carried forward coverage won't be shown. Click here to find out more.

Impacted Files Coverage Δ Complexity Δ
lib/Controller/MainController.php 0.00% <0.00%> (ø) 41.00 <0.00> (+1.00)
lib/Controller/RecipeController.php 0.00% <0.00%> (ø) 15.00 <0.00> (+1.00)
lib/Exception/RecipeExistsException.php 0.00% <0.00%> (ø) 1.00 <1.00> (?)
lib/Service/RecipeService.php 0.00% <0.00%> (ø) 241.00 <0.00> (ø)

@christianlupus
Copy link
Collaborator Author

@seyfeb I am getting some strange errors. It seems I have tomatoes on my eyes. Can you have a quick glance if you see the culprit?

I get this error:
grafik
to corelate the line numbers, here the debugger's output
grafik

Can you explain why the exception is passed on to the browser although it is caught?

@seyfeb
Copy link
Collaborator

seyfeb commented Apr 28, 2021

I pulled the PR and can't reproduce this behavior. I get the XHR error but no other. Maybe you need to clear your cache?

@christianlupus
Copy link
Collaborator Author

The issue is merely esthetical. The main functionality is given as far as I see.

In a new browser (private window), the issue persisted. Also rebuilding using npm run build did not solve it. What else could it be? The node.js version (14 for me)?

Eventually, I could try to use the zip file from the GH actions to see if this is something with my machine/build setup.

@christianlupus christianlupus marked this pull request as ready for review May 12, 2021 13:20
Signed-off-by: Christian Wolf <github@christianwolf.email>
Signed-off-by: Christian Wolf <github@christianwolf.email>
Signed-off-by: Christian Wolf <github@christianwolf.email>
Signed-off-by: Christian Wolf <github@christianwolf.email>
Signed-off-by: Christian Wolf <github@christianwolf.email>
Signed-off-by: Christian Wolf <github@christianwolf.email>
Signed-off-by: Christian Wolf <github@christianwolf.email>
Signed-off-by: Christian Wolf <github@christianwolf.email>
@christianlupus christianlupus force-pushed the fix/690-clear-message-when-recipe-exists branch from 58bb285 to 6da7ae8 Compare May 12, 2021 13:48
@christianlupus christianlupus merged commit 0c6d669 into master May 12, 2021
@delete-merged-branch delete-merged-branch bot deleted the fix/690-clear-message-when-recipe-exists branch May 12, 2021 14:05
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.

Show correct notification when recipe already exist
2 participants