More clear error message for a remixed neighborhood level - #48119
Conversation
| THEATER: 'theater' | ||
| }; | ||
|
|
||
| export function getUnsupportedMiniAppMessage(miniAppType) { |
There was a problem hiding this comment.
Wasn't sure what the best place for this helper function was. It's not a constant, so it feels a bit weird to put in this file, but it is directly related to CsaViewMode (aka mini app type) and is used externally in separate files, so this felt like one possible common spot for it. Definitely open to other ideas, so let me know if there's a better place
There was a problem hiding this comment.
I'd be in favor of making a new file named something like utils that this and potentially other methods could go into
There was a problem hiding this comment.
Good idea, I'll go ahead and do that
molly-moen
left a comment
molly-moen
left a comment
There was a problem hiding this comment.
Looks good, just a couple comments!
| THEATER: 'theater' | ||
| }; | ||
|
|
||
| export function getUnsupportedMiniAppMessage(miniAppType) { |
There was a problem hiding this comment.
I'd be in favor of making a new file named something like utils that this and potentially other methods could go into
| if (!this.seenUnsupportedNeighborhoodMessage) { | ||
| this.onOutputMessage(javalabMsg.unsupportedNeighborhoodMessage()); | ||
| this.onOutputMessage( | ||
| '[WARNING] ' + getUnsupportedMiniAppMessage(CsaViewMode.NEIGHBORHOOD) |
There was a problem hiding this comment.
should we just use [EXCEPTION] here to be consistent?
There was a problem hiding this comment.
Yeah, that's fine with me. It technically wouldn't be an exception that caused it but I don't think it makes a huge difference to the user
| if (!this.seenUnsupportedNeighborhoodMessage) { | ||
| this.onOutputMessage(javalabMsg.unsupportedNeighborhoodMessage()); | ||
| this.onOutputMessage( | ||
| '[EXCEPTION] ' + getUnsupportedMiniAppMessage(CsaViewMode.NEIGHBORHOOD) |
There was a problem hiding this comment.
should [EXCEPTION] be part of the i18n string so it's translatable?
There was a problem hiding this comment.
oh good point, we have a constant EXCEPTION_PREFIX in our constants file but it's not translatable. We should probably move that into an i18n string. We could just set the constant to equal a new i18n string so we don't have to change it everywhere I think
There was a problem hiding this comment.
Yeah good call, I'll add that
There was a problem hiding this comment.
yeah, although it gets tricky when you start concatenating i18n strings because in RTL the order would be backwards (it would be "error message [EXCEPTION]")
There was a problem hiding this comment.
Hmm yeah - FWIW we do have another string that already contains this label ("[EXCEPTION {exceptionName} was thrown") and seems like the translations don't change it (ex. in es_es.json it's "[EXCEPTION] {exceptionName} fué lanzada")? Not sure if that's something we explicitly asked for or if it just happened during the translation process.
There was a problem hiding this comment.
Looks like the EXCEPTION_PREFIX is only actually used here, so I think for now I'll create a new string with the prefix and the message ("[EXCEPTION] {details}") and use that in place of the constant. That way at least the translation will be consistent even if "EXCEPTION" doesn't end up getting translated.
3 participants
Fixes an issue where we were displaying an "invalid grid" error message when running a neighborhood project on a remixed (i.e. non-neighborhood) level. The reason here was that all exception messages, including mini-app specific exceptions, have a message type of "EXCEPTION" and therefore get routed to our exception handler rather than mini-app specific handlers. This change adds logic to the exception handler to check the mini-app type and display the correct error message.
Neighborhood project on a remixed (Theater) level:
Neighborhood project on an actual Theater level:
Theater project throwing an exception on a Neighborhood level:
Running tests with Theater exceptions on a Neighborhood level:
Running tests with Neighborhood code on a remixed (Theater) level:
Links
JIRA: https://codedotorg.atlassian.net/browse/JAVA-678
Testing story
Deployment strategy
Follow-up work
Privacy
Security
Caching
PR Checklist: