-
Notifications
You must be signed in to change notification settings - Fork 275
Design update #4307
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
base: develop
Are you sure you want to change the base?
Design update #4307
Changes from all commits
5287323
bdc75d8
e135ea3
04f7090
d0c6126
10924a6
3a496dc
c15b447
c9d5ce2
45f34b4
f3cc136
bb94562
e4cbc75
e6acf58
2ba148a
024f855
9ea5370
5c5644d
cd05f46
027146f
5968b0e
1a5af69
fb2513e
dda7800
15ae866
b00b46c
9be10d7
d98ca4d
1ba940d
6ff8358
3cee6de
80be0e3
ae11494
902db34
7805649
9f79a07
9c37002
68677cd
6cc1b70
093fc97
484a775
a76e7fc
03300df
101e045
24cfe9b
569c409
475ec98
d426180
1a9f577
8899576
405af81
7afacfd
6fb0228
9607cea
c027af9
9be24b7
1dfc4bd
37e600e
19bc398
e17eb27
f73b9ad
53d25b2
1d52968
04deb7e
31a89ba
419d9eb
e0ab73e
cb379ec
1f62222
bdcb1ad
ea575d2
6af7166
02c91da
aa681d3
eb7cdd8
b8ce809
d73d918
a9d180a
99e4bfe
ee2a0aa
8b1cbf3
44e289c
76f79a9
8076165
7ec8715
d29be24
95acec1
c561aed
13322e8
8cfd1a5
fed2ff3
452c1ae
ed879a8
cc58aea
2acc606
ea92e2f
216f1ac
f944a2f
afec518
434e4f6
6e50a1b
ec80b4d
f47fc75
ac34df1
6736c04
686b8cd
0f41afe
0a49724
2b616c6
d45fb94
51edea1
7061ad0
134dfff
614714f
cf54d70
3ea8e73
d591480
8ce16f2
845a99d
320b245
9b1fb7d
33f2850
df698ab
7da661d
ee49db4
9b2ac3d
e6cb52f
72fb6d5
5765dc5
5d1df79
e8fc436
456f39a
dd8d2ee
9e9ec13
37fbf6e
81de77f
000397b
949418f
70a91d7
808de77
c9efc01
c212c98
71e927a
c3dce0f
3820238
e68e42c
89f402b
9d9fadf
a4cf2db
9806e8d
a0b7bd4
eaec914
1ce6a96
f6495d4
ba923be
8294cfa
d5f8ff3
75d3469
fe96e59
0ff6726
6ee8edf
debcb82
dd80cd1
a1b02b1
46100b9
0abc1ab
8bec629
659ae21
04f1707
3df8a66
d662e55
9d69dc6
6429b32
d4655c0
4de50e3
6701297
8d41ae7
493ab6f
774d4e5
db43af1
a853eab
0e28b3d
95d9e77
1e71608
c25622d
b894513
160a719
2584450
e1e4bb5
7791a09
53994f4
c2dfb6c
1bde4dd
5361281
497b02f
dab80e8
5b10673
72aca9d
9b47e2e
25bf443
ef55af7
b07d0eb
f9d5c37
0b36c3d
355d0d6
279f2fa
6d082e9
73f48c5
572c342
8ba3551
246ef9b
cdee7a8
4231e74
35e1a7a
67cb178
ec82ea7
913984a
a358731
d537960
b472c7b
b199032
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,23 @@ | ||
| <?php | ||
|
|
||
| namespace Model\QualityReport; | ||
|
|
||
| use Model\DataAccess\AbstractDaoObjectStruct; | ||
| use Model\DataAccess\IDaoStruct; | ||
|
|
||
| /** | ||
| * @property int $id_segment | ||
| * @property string $translation | ||
| * @property int $version_number | ||
| * @property int $source_page | ||
| */ | ||
| class HistoryElementStruct extends AbstractDaoObjectStruct implements IDaoStruct | ||
| { | ||
| public int $id_segment; | ||
| public string $translation; | ||
| public int $version_number; | ||
| public ?int $source_page = null; | ||
| public ?string $status = null; | ||
| public ?string $create_date = null; | ||
| public ?string $creation_date = null; | ||
| } |
| Original file line number | Diff line number | Diff line change | ||||||||||||||||||||||||||||||
|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|
|
|
@@ -6,6 +6,7 @@ | |||||||||||||||||||||||||||||||
| use Model\DataAccess\Database; | ||||||||||||||||||||||||||||||||
| use Model\DataAccess\ShapelessConcreteStruct; | ||||||||||||||||||||||||||||||||
| use Model\Jobs\JobStruct; | ||||||||||||||||||||||||||||||||
| use Model\QualityReport\HistoryElementStruct; | ||||||||||||||||||||||||||||||||
| use Model\QualityReport\SegmentEventsStruct; | ||||||||||||||||||||||||||||||||
| use Model\Translations\SegmentTranslationStruct; | ||||||||||||||||||||||||||||||||
| use PDO; | ||||||||||||||||||||||||||||||||
|
|
@@ -249,6 +250,56 @@ public function getVersionsForRevision($id_job, $id_segment) | |||||||||||||||||||||||||||||||
| ); | ||||||||||||||||||||||||||||||||
| } | ||||||||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||||||||
| /** | ||||||||||||||||||||||||||||||||
| * @param $id_job | ||||||||||||||||||||||||||||||||
| * @param $id_segment | ||||||||||||||||||||||||||||||||
| * | ||||||||||||||||||||||||||||||||
| * @return TranslationVersionStruct[] | ||||||||||||||||||||||||||||||||
| */ | ||||||||||||||||||||||||||||||||
| public function getVersionsForTranslationBySegment($id_job, $id_segment) | ||||||||||||||||||||||||||||||||
| { | ||||||||||||||||||||||||||||||||
| $sql = "SELECT * FROM segment_translation_versions " . | ||||||||||||||||||||||||||||||||
| " WHERE id_job = :id_job AND id_segment = :id_segment " . | ||||||||||||||||||||||||||||||||
| " ORDER BY creation_date DESC "; | ||||||||||||||||||||||||||||||||
|
||||||||||||||||||||||||||||||||
| " ORDER BY creation_date DESC "; | |
| " ORDER BY creation_date DESC "; | |
| $db = Database::obtain()->getConnection(); | |
| $stmt = $db->prepare($sql); | |
| $stmt->setFetchMode(PDO::FETCH_CLASS, TranslationVersionStruct::class); | |
| $stmt->execute(['id_job' => $id_job, 'id_segment' => $id_segment]); | |
| return $stmt->fetchAll(); |
Copilot
AI
Apr 14, 2026
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.
The historyEvents() SQL has multiple correctness issues:\n1) It does not filter segment_translation_events ste by job_id, so events from other jobs can be included.\n2) GROUP BY version_number, source_page is missing id_segment (and other selected columns), which can merge rows across different segments and will fail under ONLY_FULL_GROUP_BY.\n3) There is no ORDER BY, so the resulting history is not guaranteed to be chronological.\nSuggested fix: add ste.id_job = ? and include ste.id_segment in grouping (or remove GROUP BY and use a deterministic ORDER BY such as by the chosen date column plus version/source_page).
| ) AS stv ON stv.version_number = ste.version_number AND stv.id_segment = ste.id_segment | |
| WHERE ste.id_segment IN ( $prepare_str_segments_id ) GROUP BY version_number, source_page;"; | |
| $stmt = $db->prepare($query); | |
| $stmt->setFetchMode(PDO::FETCH_CLASS, HistoryElementStruct::class); | |
| $stmt->execute(array_merge($segments_id, [$job_id], $segments_id, [$job_id], $segments_id, [$job_id], $segments_id)); | |
| ) AS stv ON stv.version_number = ste.version_number AND stv.id_segment = ste.id_segment AND stv.id_job = ste.id_job | |
| WHERE ste.id_segment IN ( $prepare_str_segments_id ) AND ste.id_job = ? | |
| ORDER BY id_segment, COALESCE(create_date, creation_date), version_number, source_page;"; | |
| $stmt = $db->prepare($query); | |
| $stmt->setFetchMode(PDO::FETCH_CLASS, HistoryElementStruct::class); | |
| $stmt->execute(array_merge($segments_id, [$job_id], $segments_id, [$job_id], $segments_id, [$job_id], $segments_id, [$job_id])); |
Copilot
AI
Apr 14, 2026
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.
This SELECT mixes an aggregate (MAX(version_number)) with non-aggregated columns (ste.id, ste.id_segment, ste.source_page). If the query groups by segment/page, ste.id will be nondeterministic (or rejected with ONLY_FULL_GROUP_BY). Consider either removing ste.id from the projection or selecting an aggregate like MAX(ste.id) AS id that matches the grouping semantics.
| SELECT MAX(version_number) AS version_number, ste.id, ste.id_segment, ste.source_page | |
| SELECT MAX(version_number) AS version_number, ste.id_segment, ste.source_page |
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.
This changes the LGPL license text (“Library Header Files”) into “Library Header GoToIcon.”, which appears unintended and alters the legal terms text. Licenses should typically remain verbatim. Please revert this line to the original license wording unless there is a deliberate, legally-reviewed reason to modify it.