joshwcomeau / guppy

🐠A friendly application manager and task runner for React.js
ISC License
3.27k stars 154 forks source link

Show dialog if project delete fails #304

Closed melanieseltzer closed 5 years ago

melanieseltzer commented 5 years ago

Related Issue:

Fixes #279

Summary:

Show a dialog to the user if the project delete fails for some reason.

I ended up wrapping rimraf + moveItemToTrash into a try/catch because I was getting uncaught at callee if I simply rejected with false. Not sure if this is good or if it should be structured differently.

Also, not sure I like that the fish spinner freezes while waiting for the dialog to get clicked. Is there a way around that or is that how it should be?

To do

Screenshots

kapture 2018-10-23 at 21 20 14

melanieseltzer commented 5 years ago

@AWolf81 Changing to read only was the only way for me to force a failure (cd'ing into the project folder or node_modules in terminal would not fail! heh).

Can you test this on Windows to see if it addresses the failure you were experiencing?

melanieseltzer commented 5 years ago

Awesome, just having trouble fixing the logs an error when the project can't be deleted test. Do you know how to force a catch in Jest?

AWolf81 commented 5 years ago

Yes, you can use saga.throw() to test the catch block as mentioned here.

I've fixed it in branch fix-test-delete-project-saga. I'll create a PR to your branch in a minute. Just one point was a bit weird - I needed to use JSON.stringify for the loadDependencyInfoFromDisk test. Feels hacky but it's working. It told something like compared values have no visual difference - similar to this SO question

PR #308 should fix the test.

codecov[bot] commented 5 years ago

Codecov Report

Merging #304 into master will increase coverage by 0.12%. The diff coverage is 91.66%.

Impacted file tree graph

@@            Coverage Diff             @@
##           master     #304      +/-   ##
==========================================
+ Coverage   20.49%   20.61%   +0.12%     
==========================================
  Files         239      239              
  Lines        3733     3740       +7     
  Branches      385      385              
==========================================
+ Hits          765      771       +6     
- Misses       2690     2691       +1     
  Partials      278      278
Impacted Files Coverage Δ
src/reducers/app-status.reducer.js 57.14% <ø> (ø) :arrow_up:
src/actions/index.js 94.49% <100%> (+0.1%) :arrow_up:
src/sagas/delete-project.saga.js 78.94% <90%> (+0.15%) :arrow_up:
melanieseltzer commented 5 years ago

@AWolf81 Brilliant, TIL about saga.throw. Cool stuff 👍 Thanks for fixing it! So this is ready to merge right?