nukeop / nuclear

Streaming music player that finds free music for you
https://nuclear.js.org/
GNU Affero General Public License v3.0
11.72k stars 1.02k forks source link

Fix - #1582 - Canceling importing JSON #1597

Closed BMischley closed 2 months ago

BMischley commented 2 months ago

Fixing #1582

Created default value and check to no longer create error shown in issue.

nuki-chan[bot] commented 2 months ago

Lines 154-156

Nuki has spotted a teensy little snafu here, folks! 😸🌟✨ Looks like you're trying to handle an empty filePath, which is super cute but a bit misplaced. It's like when you give someone a high five, and they just leave you hanging—ouch! 😹

So, let's break it down:

Here's how Nuki suggests spicing it up! Create a proper check for the filePath before you even attempt to read it. If it's not there, dispatch that error like you're launching a cute rocket. 🚀💥

export function addPlaylistFromFile(filePath, t) {
  return async dispatch => {
    if (!filePath || typeof filePath !== 'string') {
      dispatch(error(t('import-fail-title'), t('error-no-filepath'), null, null));
      return;
    }
    // Read the file...
  };
}

Remember, humans, this is just Nuki's advice, not the ultimate command from the coding anime universe. 😉 You humans still have the freedom to blend it with your own logic magic! ✨


Lines 155-156

Okay, let's hit pause for a second. That sneaky if statement just waltzed in without any braces, leaving poor return; out in the cold. That's no fun, right? 🥶

While it might seem all cool and compact, let's remember our curly braces {}. They're like your favorite anime character's signature outfit—you don't want to see them without it! So here's the outfit your code deserves:

if (filePath === '') {
  dispatch(error(t('import-fail-title'), t('error-empty-filepath'), null, null));
  return;
}

Much better, neh? ✌️ Remember, consistency is key in the code dojo! 👩‍💻🥋

nukeop commented 2 months ago

Thanks, looks ok. It would be nice to have a test for this, do you think you could write one?

BMischley commented 2 months ago

Sure @nukeop . I have finished writing a test, but am having issues actually running the test/verifying the test works. It appears they are failing to run, any ideas? Additionally, would PlaylistsContainer.test.tsx be the best location for this test?

Thanks!

Screenshot 2024-04-24 at 6 52 18 PM
nukeop commented 2 months ago

Looks like a wrong type somewhere. Your editor should highlight wher eit occurs (line 55?)

BMischley commented 2 months ago

Interesting... I didn't edit anything in any of those files. I'll do a bit of digging.

BMischley commented 2 months ago

@nukeop is there anything else you'd suggest I do?

nukeop commented 2 months ago

Everything looks great, thank you!