Skip to content

fix: improve error message formatting in setSynthVolume#6698

Closed
CodeBySayak wants to merge 4 commits into
sugarlabs:masterfrom
CodeBySayak:error-msg
Closed

fix: improve error message formatting in setSynthVolume#6698
CodeBySayak wants to merge 4 commits into
sugarlabs:masterfrom
CodeBySayak:error-msg

Conversation

@CodeBySayak

Copy link
Copy Markdown
Contributor

Description

This PR fixes a malformed error message in the setSynthVolume method within js/turtleactions/VolumeActions.js.

The Problem

When the setSynthVolume function fails to identify a synthesizer or instrument name, it attempts to display an error message to the user. However, the existing code was using the synth variable—which is explicitly null at that point in the logic—instead of the actual input string (synthname). Additionally, the message lacked a space and proper localization support.

Original Buggy Output: nullnot found

Changes

  • Updated the error message to use synthname (the actual user-provided input).
  • Added a space for readability.
  • Wrapped the "not found" text in the _() translation helper to ensure it is correctly localized for all users.

Testing

  • Verified that unrecognized instrument names now trigger a readable and properly formatted error alert identifying exactly which input was not found.

Type of Change

  • Bug fix (non-breaking change which fixes an issue)
  • New feature
  • Breaking change
  • Documentation update

closes #6697

@github-actions github-actions Bot added bug fix Fixes a bug or incorrect behavior size/XS Extra small: < 10 lines changed area/javascript Changes to JS source files labels Apr 18, 2026
@github-actions

Copy link
Copy Markdown
Contributor

❌ Some Jest tests failed. Please check the logs and fix the issues before merging.

Failed Tests:

phrasemaker.test.js
VolumeActions.test.js

@github-actions github-actions Bot added the area/tests Changes to test files label Apr 18, 2026
@github-actions

Copy link
Copy Markdown
Contributor

❌ Some Jest tests failed. Please check the logs and fix the issues before merging.

Failed Tests:

phrasemaker.test.js

@CodeBySayak CodeBySayak changed the title fixed error msg fix: improve error message formatting in setSynthVolume Apr 18, 2026
@Ashutoshx7

Copy link
Copy Markdown
Member

hy jest is faling take a look

@github-actions

Copy link
Copy Markdown
Contributor

❌ Some Jest tests failed. Please check the logs and fix the issues before merging.

Failed Tests:

phrasemaker.test.js

@Ashutoshx7

Copy link
Copy Markdown
Member

jest faling still

@github-actions github-actions Bot added size/S Small: 10-49 lines changed and removed size/XS Extra small: < 10 lines changed labels Apr 18, 2026
@github-actions

Copy link
Copy Markdown
Contributor

✅ All Jest tests passed! This PR is ready to merge.

Coverage: Coverage summary unavailable

@CodeBySayak

Copy link
Copy Markdown
Contributor Author

@Ashutoshx7 can you review again I have passed jest tests

@walterbender

Copy link
Copy Markdown
Member

Please put the phrasemaker changes into a separate PR

@github-actions github-actions Bot added size/XS Extra small: < 10 lines changed and removed size/S Small: 10-49 lines changed labels Apr 18, 2026
@github-actions

Copy link
Copy Markdown
Contributor

❌ Some Jest tests failed. Please check the logs and fix the issues before merging.

Coverage: Coverage summary unavailable

Failed Tests:

phrasemaker.test.js

@github-actions

Copy link
Copy Markdown
Contributor

This PR has merge conflicts with master.

Please rebase your branch:

# Add upstream remote (one-time setup)
git remote add upstream https://github.com/sugarlabs/musicblocks.git

# Fetch latest master and rebase
git fetch upstream
git rebase upstream/master

# Resolve any conflicts, then:
git push --force-with-lease origin YOUR_BRANCH

Tip: Enable "Allow edits from maintainers" on this PR so we can auto-rebase for you next time. This only grants access to your PR branch. Your fork's other branches are not affected.

@github-actions

Copy link
Copy Markdown
Contributor

❌ Some Jest tests failed. Please check the logs and fix the issues before merging.

Coverage: Coverage summary unavailable

Failed Tests:

phrasemaker.test.js

@github-actions

Copy link
Copy Markdown
Contributor

Merge conflicts resolved. Ready for review.

@github-actions

Copy link
Copy Markdown
Contributor

❌ Some Jest tests failed. Please check the logs and fix the issues before merging.

Coverage: Statements: 41.9% | Branches: 34.79% | Functions: 46.55% | Lines: 42.3%

Failed Tests:

phrasemaker.test.js

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

area/javascript Changes to JS source files area/tests Changes to test files bug fix Fixes a bug or incorrect behavior size/XS Extra small: < 10 lines changed

Projects

None yet

Development

Successfully merging this pull request may close these issues.

[Bug] Correct malformed error message in setSynthVolume

3 participants