Module talk:Commons link

Code review?

@RexxS: Remember the discussion we had on Mike Peel's talk page? I've implement Lua code to perform cross-checking against multiple Wikidata properties to look for Commons galleries or categories (or both). That is this module, which is currently in use at {{commons and category}} and {{commons and category-inline}}. I'd like to use it for {{commons-inline}}, and perhaps {{commons category}}. Before I do anything, would you be willing to look at the code and give me feedback? As usual, the testcases are here. Thanks for any help or suggestions you can provide! — hike395 (talk) 15:48, 15 March 2020 (UTC)

@Hike395: I've just checked the results at Module talk:Commons link/testcases and they look reasonably comprehensive, and absolutely accurate. I've also now gone through the code at Module:Commons link and it looks good – a lot of the code is similar to functions that I've used in the past, so I'm confident that your algorithms are correct. The only problems I foresee are when editors manage to misuse the code, and you need to decide whether it is better to let a function fail with a sensible error message or catch the error and let the function continue with a null result. For example:

• {{#invoke:Commons link |getGallery |qid=Q0 |search=}} → (← bad entity-id)
• {{#invoke:Commons link |getGallery |qid=Q68979196 |search=}} → [[Commons:Special:Search/|]] (← entity-id exists, but has no links)

You'll usually have to work out in your own mind whether it's possible to abuse the local functions, such as at line 15 qid = mw.wikibase.getEntityIdForCurrentPage() – is it possible for the code to be running on a page that has no linked EntityID? If so, what happens when qid is returned as nil on line 16? Similarly for line 20, if mw.wikibase.sitelink(qid) somehow manages to return nil (no sitelink). Incidentally, I'd recommend using the latest calls: mw.wikibase.getSitelink is current, while mw.wikibase.sitelink is a legacy alias and shouldn't be used in new code (per mw:Extension:Wikibase Client/Lua #Legacy aliases).

Anyway, I'd have no qualms about introducing the code into production, as long as you're ready to catch any feedback from editors who spot errors. To paraphrase von Moltke, "No code survives contact with the user." Cheers --RexxS (talk) 16:51, 15 March 2020 (UTC)

@RexxS: Thanks for the code review! Most of the code has nil checks (because I copied a lot of code fragments from your code in Module:WikidataIB). I've cleaned up the remaining problems that I could find, and added tests to cover the problems you found. Next, I'll propose use at Template talk:Commons-inline. Thanks again! — hike395 (talk) 00:40, 16 March 2020 (UTC)

@Hike395: I don't know enough Lua to review the code, but I'd like to suggest a different way to go about this. In an ideal world, I think we would want the following behaviour:

1. If there is a gallery link, show that.
2. If there is a category link, show that (in addition to the gallery link if both are available).
3. If there are additional category links, such as through category for the interior of the item (P7561), then also show those.
4. If not, show the search link.
5. With tracking categories to say which options are being used, and extra tracking categories if they are locally defined.

I think that means merging {{Commons}}, {{Commons category}} and {{Commons and category}}, which would make things simpler here if there is a good Lua back-end to run them. Personally, I'd prefer we drop support for Commons category (P373) and Commons gallery (P935) as early as we can, in favour of the sitelinks, but keeping fall-back options for now seems OK. Thanks. Mike Peel (talk) 21:34, 17 March 2020 (UTC)

@Mike Peel and RexxS: The functions in the module can certainly be used to create what you suggest. I very much like the combination of (1), (2) and (4). It's pretty close to what I want to do at {{Commons-inline}}, except I was planning on showing either a gallery or a category link, and you're proposing showing both if they both exist. Probably the easiest way to proceed is to change the behavior {{Commons and category}} and {{Commons and category-inline}} to reflect your suggestion.

Note that in order to do both (1) and (2), you'll probably need to keep both Commons category (P373) and Commons gallery (P935), because you need those to complement the Commons sitelink, which can only store the gallery or the category, but not both. — hike395 (talk) 01:11, 18 March 2020 (UTC)

@Mike Peel and RexxS: For fun, I implemented part of Mike's idea [(1), (2), and (4)]. It was quite easy: I put it into getGalleryAndCategory() in the sandbox module, and updated {{Commons and category/sandbox}} to use it. If we decide to adopt this, it will change the behavior of {{Commons and category}} if there is exactly one of a gallery and a category -- instead of searching for the other one, it will just present one. If there are neither, it will offer a search for both.

For extra credit, I wrote this in such a way that {{Commons}} and {{Commons category}} can become wrappers around {{Commons and category}}. {{Commons}} behavior would change (a category link might show up). {{Commons category}} behavior would change (a gallery link might show up). But it should work.

WDYT? — hike395 (talk) 04:20, 18 March 2020 (UTC)

@Hike395: I like this, hopefully the wider community will too. With the sitelinks to both gallery and category, the standard setup on wikidata is like that at Czarnca (Q2322184) (a random example, the last one I was editing yesterday). There you can get the gallery sitelink from Czarnca (Q2322184), and follow topic's main category (P910) through to Category:Czarnca (Q87840009) to get the category sitelink. You'd need to include a namespace check on that, as in categories you'd have to do the reverse via category's main topic (P301). No need to keep Commons category (P373) and Commons gallery (P935) in that case (others I'm still working through to clean them up though!). Thanks. Mike Peel (talk) 09:16, 18 March 2020 (UTC)

Lua error

Some articles are showing "Lua error: bad argument #1 to 'getBestStatements' (string expected, got nil)". Examples: Pastry, Remote control. That is because qid is nil in mw.wikibase.getBestStatements(qid, "P373") at line 184. I'm hoping Hike395 will investigate. Johnuniq (talk) 10:28, 2 August 2021 (UTC)

I know what it is. Preparing a fix now. — hike395 (talk) 12:56, 2 August 2021 (UTC)

 Fixed --- I added a test case to check Pastry. — hike395 (talk) 13:11, 2 August 2021 (UTC)

Edit request 8 September 2024

Hi, could you please apply this change to the main module?

Od1n (talk) 11:55, 8 September 2024 (UTC)

 Done — Martin (MSGJ · talk) 20:05, 9 September 2024 (UTC)