storj icon indicating copy to clipboard operation
storj copied to clipboard

web/multinode: Show appropriate error while adding new node

Open Pranav2612000 opened this issue 3 years ago • 8 comments

While adding new nodes, if the nodeID, Public IP Address, or the API Key is incorrect no error was being shown on the webpage. This commit resolves this issue by showing an error notification on the top right corner of the screen with appropriate error text.

Fixes #4199

Please describe the performance impact:

Code Review Checklist (to be filled out by reviewer)

  • [ ] NEW: Are there any Satellite database migrations? Are they forwards and backwards compatible?
  • [ ] Does the PR describe what changes are being made?
  • [ ] Does the PR describe why the changes are being made?
  • [ ] Does the code follow our style guide?
  • [ ] Does the code follow our testing guide?
  • [ ] Is the PR appropriately sized? (If it could be broken into smaller PRs it should be)
  • [ ] Does the new code have enough tests? (every PR should have tests or justification otherwise. Bug-fix PRs especially)
  • [ ] Does the new code have enough documentation that answers "how do I use it?" and "what does it do?"? (both source documentation and higher level, diagrams?)
  • [ ] Does any documentation need updating?
  • [ ] Do the database access patterns make sense?

Pranav2612000 avatar Nov 24 '21 15:11 Pranav2612000

Hey! This is my first PR to this project so let me know if anything is missing! Thank you.

Pranav2612000 avatar Nov 24 '21 15:11 Pranav2612000

Hey @Pranav2612000, thank you for your pull request :)

Before I review this change, could you get the build to pass? It looks like there are currently some linting errors: https://build.dev.storj.io/blue/organizations/jenkins/storj/detail/PR-4293/1/pipeline/ By the way, you can see the outcome of future builds by clicking "Details" next to "continuous-integration/jenkins/pr-head — This commit cannot be built" at the bottom of your PR

mobyvb avatar Nov 24 '21 16:11 mobyvb

Thank you for your pull request and welcome to our community. We require contributors to sign our Contributor License Agreement, and we don't seem to have the users @Pranav2612000 on file. Once you have signed the CLA, please let us know, so we can manually review and add you to the approved contributors list.

cla-bot[bot] avatar Nov 24 '21 18:11 cla-bot[bot]

@Pranav2612000 , one more thing: could you upgrade your npm version to at least version 7? For reference, my npm/node versions are 7.5.1/15.8.0 (you shouldn't need those exact versions). But we think part of the reason why so many lines were removed from your package-lock.json is that you have an older version of npm.

mobyvb avatar Nov 24 '21 23:11 mobyvb

Thank you for your pull request and welcome to our community. We require contributors to sign our Contributor License Agreement, and we don't seem to have the users @Pranav2612000 on file. Once you have signed the CLA, please let us know, so we can manually review and add you to the approved contributors list.

cla-bot[bot] avatar Nov 25 '21 14:11 cla-bot[bot]

Hey @mobyvb ! I've updated nvm and node to 7.5.1/15.8.0 as you asked.

Pranav2612000 avatar Nov 25 '21 14:11 Pranav2612000

@cla-bot check

stefanbenten avatar Nov 25 '21 15:11 stefanbenten

The cla-bot has been summoned, and re-checked this pull request!

cla-bot[bot] avatar Nov 25 '21 15:11 cla-bot[bot]

This pull request has been mentioned on Storj Community Forum (official). There might be relevant details there:

https://forum.storj.io/t/multinode-seems-to-stop-where-it-should-continue/21697/14

storjrobot avatar May 22 '23 16:05 storjrobot