CLDR-17461 Improve visibility of forum posts in vetting view

[btangmu]

-New icons in the Comparison (English) column: 💬 followed by question mark or period

-💬? means: This item has forum posts, some of which are open

-💬. means: This item has forum posts, all of which are closed

-New class SurveyForum.PathForumStatus

-New method appendForumStatus in cldrTable.mjs

-Fix a few Java compiler warnings

CLDR-17461

• This PR completes the ticket.

ALLOW_MANY_COMMITS=true

[macchiati]

Looks good to me, but SRL should review.

[srl295]

I like how the UI was hooked in! Looks fine, on the SQL perf it's the round trip that's a challenge. So probably the two queries could be a single one that would be just as fast. Also this is a separate query per XPath which will be expensive. Cheaper would probably be something like "list of xpaths in this locale and a count of posts and open posts" and just invalidate it if someone calls the post endpoint.

Example - this is on production

SET SESSION TRANSACTION READ ONLY; -- safety first!
select xpath, count(*), max(is_open) from cldr_forum_posts where loc='fr' group by xpath;

...

| 698928 |        8 |            0 |
| 698931 |        3 |            0 |
| 698932 |        4 |            0 |
| 698936 |        3 |            0 |
| 699740 |        2 |            1 |
| 699761 |        4 |            0 |
| 699771 |        1 |            0 |
| 699772 |        7 |            1 |
| 699778 |        8 |            1 |
+--------+----------+--------------+
886 rows in set (0.00 sec)

even the entire set is pretty speedy

| zu         | 686547 |        2 |            0 |
| zu         | 698722 |        2 |            0 |
| zu         | 698781 |        2 |            0 |
| zu         | 698863 |        2 |            0 |
+------------+--------+----------+--------------+
51444 rows in set (0.65 sec)

but each sql connection for each xpath adds overhead.

[macchiati]

The suggestion for list of xpaths in this locale and a count of posts and open posts (invalidating if stale) is clever

…

On Wed, May 1, 2024 at 2:56 PM Steven R. Loomis ***@***.***> wrote: ***@***.**** approved this pull request. I like how the UI was hooked in! Looks fine, on the SQL perf it's the round trip that's a challenge. So probably the two queries could be a single one that would be just as fast. Also this is a separate query per XPath which will be expensive. Cheaper would probably be something like "list of xpaths in this locale and a count of posts and open posts" and just invalidate it if someone calls the post endpoint. Example: select xpath, count(*), max(is_open) from cldr_forum_posts where loc='fr' group by xpath; ... | 698928 | 8 | 0 | | 698931 | 3 | 0 | | 698932 | 4 | 0 | | 698936 | 3 | 0 | | 699740 | 2 | 1 | | 699761 | 4 | 0 | | 699771 | 1 | 0 | | 699772 | 7 | 1 | | 699778 | 8 | 1 |+--------+----------+--------------+886 rows in set (0.00 sec) — Reply to this email directly, view it on GitHub <#3675 (review)>, or unsubscribe <https://github.com/notifications/unsubscribe-auth/ACJLEMH3YQ5DOYU4Z4MQOIDZAFQJTAVCNFSM6AAAAABHCKDYKOVHI2DSMVQWIX3LMV43YUDVNRWFEZLROVSXG5CSMV3GSZLXHMZDAMZUGUZDANRWGA> . You are receiving this because your review was requested.Message ID: ***@***.***>

[AEApple]

Where can I preview this? I tried in staging but I didn't see an icon for some closed forum posts. Do I need to check smoke test?

[btangmu]

I went ahead and merged this. @AEApple within an hour it should be running on cldr-smoke so you can test and evaluate the interface. @srl295 and @macchiati I agree the db access probably needs optimization, though I was considering first to measure hot spots and confirm that it's a hot spot. A cache for each locale may be expensive in terms of memory, though it's good that only the paths with posts need caching...

[srl295]

I went ahead and merged this. @AEApple within an hour it should be running on cldr-smoke so you can test and evaluate the interface. @srl295 and @macchiati I agree the db access probably needs optimization, though I was considering first to measure hot spots and confirm that it's a hot spot. A cache for each locale may be expensive in terms of memory, though it's good that only the paths with posts need caching...

Thousands of sql calls is also expensive.

But 100% on moving forward and improve later

[macchiati]

One suggestion for improvement. The question vs period is a small difference when scanning. I suggest instead of gray icon and black ? / . — use green for closed forums, and orange for open forums (keeping the white interior). (That will also not mix text with icons) Alternatively, if you want to keep using emoji, - use 💬 for closed forums - use 👁️‍🗨️ for open forums. Not a blocker.

…

On Thu, May 2, 2024 at 9:14 AM Steven R. Loomis ***@***.***> wrote: I went ahead and merged this. @AEApple <https://github.com/AEApple> within an hour it should be running on cldr-smoke so you can test and evaluate the interface. @srl295 <https://github.com/srl295> and @macchiati <https://github.com/macchiati> I agree the db access probably needs optimization, though I was considering first to measure hot spots and confirm that it's a hot spot. A cache for each locale may be expensive in terms of memory, though it's good that only the paths with posts need caching... Thousands of sql calls is also expensive. But 100% on moving forward and improve later — Reply to this email directly, view it on GitHub <#3675 (comment)>, or unsubscribe <https://github.com/notifications/unsubscribe-auth/ACJLEMDIQINSG3YMOL4VPZTZAJQ6XAVCNFSM6AAAAABHCKDYKOVHI2DSMVQWIX3LMV43OSLTON2WKQ3PNVWWK3TUHMZDAOJQHEZTSMJRHA> . You are receiving this because you were mentioned.Message ID: ***@***.***>