Problem/Motivation
Steps to reproduce:
1. Create a translatable entity (node)
2. Create a job to translate the created entity
3. Delete the original entity
4. Go to job, translate the content, review the changes of a job item, click "Save"
5. You should get a fatal error: Fatal error: Call to a member function toString() on null (JobItemForm.php on line 349)
Proposed resolution
As JobItem::getSourceUrl() can return NULL, we should (at least) check if it is not null before calling other methods on it.
| Comment | File | Size | Author |
|---|---|---|---|
| #8 | check_if_source_url_of-2690037-8-interdiff.txt | 1.39 KB | mbovan |
| #8 | check_if_source_url_of-2690037-8.patch | 3.7 KB | mbovan |
| #5 | check_if_source_url_of-2690037-5-interdiff.txt | 1.16 KB | mbovan |
| #5 | check_if_source_url_of-2690037-5.patch | 2.31 KB | mbovan |
| #2 | check_if_source_url_of-2690037-2.patch | 1.15 KB | mbovan |
Comments
Comment #2
mbovan commentedFix.
Comment #3
mbovan commentedComment #4
berdirdo we need the same for the needs review message?
Comment #5
mbovan commentedIn JobItem::needsReview() there is already a check for the source url.
Btw, first 4 steps apply for "Preview" as well - where we get an exception. Maybe we can fix it in this issue?
We could throw a 404 in case ContentTranslationPreviewController::preview() fails to load an entity...
Comment #6
berdirThrowing a not found exception probably makes most sense for that.
Comment #7
berdirWhich you already do. Changes look fine, lets make sure we have tests for it.
Comment #8
mbovan commentedAdded tests.
Comment #10
berdirWorks for me. The message is a bit strange when the source doesn't exist anymore, we should consider a more obvious warning/error. We have #1686544: Check for source change on review and show diff, please leave a comment there to care about this case too or open an issue for that.