Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

SAK-50560 Samigo fix matching feedback display to student when instru… #12943

Merged
merged 2 commits into from
Oct 4, 2024

Conversation

ottenhoff
Copy link
Contributor

…ctor does not release student answer

</h:graphicImage>
<h:column rendered="#{delivery.feedback eq 'true' && delivery.feedbackComponent.showCorrectResponse && !delivery.noFeedback=='true'}">
<h:panelGroup id="image" rendered="#{matching.isCorrect eq 'true'}" styleClass="si si-check-lg" > </h:panelGroup>
<h:panelGroup id="ximage" rendered="#{not empty matching.isCorrect && matching.isCorrect eq 'false'}" styleClass="si si-remove feedBackCross"> </h:panelGroup>
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

i guess matching.isCorrect eq 'false' is all that is needed right @ern? it is a Boolean behind the scenes so it can be null

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think if it's a Boolean and not a boolean you may have a not empty matching.isCorrect on both of these conditions to avoid an NPE? I'm not too sure about JSF.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

no doesn't seem to NPE

the root of the problem seems to be this: !matching.isCorrect which seems to match null and false

width="16" height="16"
alt="#{deliveryMessages.alt_incorrect}" url="/images/delivery/spacer.gif">
</h:graphicImage>
<h:column rendered="#{delivery.feedback eq 'true' && delivery.feedbackComponent.showCorrectResponse && !delivery.noFeedback=='true'}">
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I'm not sure if it's another issue but it seems like this should also be && delivery.noFeedback ne 'true' to be consistent with the delivery.feedback? Maybe this isn't a bug though but using eq and == doesn't seem correct.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

yes the mixing is puzzling! plus might as well be consistent and make delivery.feedbackComponent.showCorrectResponse eq 'true' also!

@ottenhoff
Copy link
Contributor Author

thanks @jonespm i revised per comments

@ottenhoff ottenhoff merged commit 2f8d957 into sakaiproject:master Oct 4, 2024
5 checks passed
ern pushed a commit that referenced this pull request Oct 8, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants