-
-
Notifications
You must be signed in to change notification settings - Fork 29
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
Fix broken fields #1718
Fix broken fields #1718
Conversation
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Wow, really impressed that you dug in to actually fix this problem, @myieye ! I am fairly confident that this has been broken for ages, however when I removed the "silently swallow all exceptions" code, it suddenly started showing the error in the client instead of failing silently.
Looks like the unit tests need tweaking with this change. Otherwise LGTM.
So, I went to expand the tests to cover the missing So...I guess it's a mistake that we show it in the UI.
|
Testingentry's import residue ❌ (FYI: @myieye )meaning's status ✅ |
@longrunningprocess thanks for testing! I had updated the issue, but forgotten to update the PR description: This PR no longer fixes the import-residue field, because it probably shouldn't exist. Your testing is green for what I did fix, so I sneakily moved this to POSO. |
Fixes #1717
Description
The
LexEntryModel->entryImportResidue and(see: #1720) LexSense->status fields throw exceptions when they're updated, because they're not configured correctly in PHP.Checklist
I have commented my code, particularly in hard-to-understand areasI have added tests that prove my fix is effective or that my feature worksQA testing
Testers, use the following instructions on our staging environment. Post your findings as a comment and include any meaningful screenshots, etc.