page does not exist removed from username #3620

318anushka · 2020-04-03T16:47:35Z

Description (required)
In Explore, some pictures have (page does not exist) string appended to the username.

Fixes #3341 "Page does not exist" near some usernames in Explore
What changes did you make and why?
formatted the string in Media getArtist() when author name is fetched through metadata.

Tests performed (required)

Tested ProdDebug on OnePlus 5 with API level 28.

tests-checker

Could you please add tests to make sure this change works as expected?

macgills · 2020-04-06T11:09:26Z

This seems a bit hacky, if we could address the root problem I would be happier. Is this somehow related to custom author names?

318anushka · 2020-04-06T12:49:19Z

The string gets appended client side i think. I have checked and couldn't find anything that generates the string in code.
Here check this
https://commons.wikimedia.org/w/api.php?action=query&format=json&formatversion=2&prop=imageinfo&iiprop=url|extmetadata&iiurlwidth=640&iiextmetadatafilter=DateTime|Categories|GPSLatitude|GPSLongitude|ImageDescription|DateTimeOriginal|Artist|LicenseShortName|LicenseUrl&titles=File%3AEilat%20Dolphin%20Reef%20%283%29.jpg

Also i found same approach in DeleteHelper

macgills · 2020-04-06T12:57:12Z

              "Artist": {
                "value": "<a href=\"//commons.wikimedia.org/w/index.php?title=User:%D7%98%D7%9C_%D7%A9%D7%9E%D7%A2&amp;action=edit&amp;redlink=1\" class=\"new\" title=\"User:טל שמע (page does not exist)\">טל שמע</a>",
                "source": "commons-desc-page"
              },

I am sorry, you might have to spell this out for me a bit more.

The API is returning this bad text, do we know why this text was created? Have you tried an upload with a bad custom author name?

The solution may be client side formatting but I would massively prefer if it wasn't

318anushka · 2020-04-06T13:10:00Z

No idea about it and it's not always created.
Sorry but I’m not very clear what do mean by bad custom author name?

macgills · 2020-04-06T13:17:59Z

In the settings screen we can put in a custom author name. #3415 #3205 #3536 there are some issues around it. I don't have enough in depth knowledge about it to confirm what is happening but if you could investigate if your uploads are tainted with a custom author name set then that may give us more insight into this problem

318anushka · 2020-04-06T14:22:25Z

macgills · 2020-04-06T14:38:53Z

Is the custom name in anyway related is what I wanted to get at.

              "Artist": {
                "value": "<a href=\"https://commons.wikimedia.beta.wmflabs.org/w/index.php?title=User:Qwert._qwerty&amp;action=edit&amp;redlink=1\" class=\"new\" title=\"User:Qwert. qwerty (page does not exist)\">qwert.  qwerty</a>",
                "source": "commons-desc-page"
              },

Is (page does not exist) added to every artist in explore? Only ones with custom names? Without custom names? Do custom names matter at all to this bug? What does this look like on any other client?

I want a complete diagnosis of the problem

318anushka · 2020-04-06T16:43:18Z

I found the cause it happens only for users who don't have a user page on commons or user not registered. So for each custom author name that user chooses he has to create a separate page i otherwise link is red.
this is related issue #3160

318anushka · 2020-04-06T22:10:20Z

do we intend to direct user to new user page with custom author name?
If no then why are user and author both set to same value. This is what creating problem i guess.

sivaraam · 2020-04-07T09:04:28Z

do we intend to direct user to new user page with custom author name?

For this issue, just forget about the Custom author name. It's not specific to that. AFAIK, you'll get that "(page does not exist)" text only when there is no user page associated with the author field. See

https://commons.wikimedia.beta.wmflabs.org/w/api.php?action=query&format=json&prop=imageinfo&titles=File%3ADalgona_coffee.jpg&formatversion=2&iiprop=url%7Cextmetadata&iiextmetadatafilter=DateTime%7CCategories%7CGPSLatitude%7CGPSLongitude%7CImageDescription%7CDateTimeOriginal%7CArtist%7CLicenseShortName%7CLicenseUrl

If we just need the name of the user who uploaded the image (as opposed to the author of the image) I believe adding the user via iiprop would give us that information. Check if that works as a possible solution. Example:

https://commons.wikimedia.beta.wmflabs.org/w/api.php?action=query&format=json&prop=imageinfo&titles=File%3ADalgona_coffee.jpg&formatversion=2&iiprop=url%7Cextmetadata%7Cuser&iiextmetadatafilter=DateTime%7CCategories%7CGPSLatitude%7CGPSLongitude%7CImageDescription%7CDateTimeOriginal%7CArtist%7CLicenseShortName%7CLicenseUrl

(See the 'user' parameter within pages > 0 > imageinfo > 0)

318anushka · 2020-04-07T09:16:23Z

If we just need the name of the user who uploaded the image (as opposed to the author of the image) I believe adding the user via iiprop would give us that information. Check if that works as a possible solution.

But in that case wouldn't be author of image on web browser be same as username?
We don't want that right else what is the use of adding custom author.

sivaraam · 2020-04-07T09:34:00Z

But in that case wouldn't be author of image on web browser be same as username?

I couldn't understand what you mean by this. Can you clarify?

sivaraam · 2020-04-07T09:40:13Z

FYI: I was suggesting that the user field might provide the desired value as the thing we're trying to fix here is just about the uploader and not about the author.

I say this based on the above screenshot which says "Uploaded by" and haven't yet looked at the code. If we use actually show the name of the author as "Uploaded by" that would mean the phrase "Uploaded by" is actually misleading as the uploader and author could be totally different for an image.

Apologies, for the vagueness of my comments. I'm just sharing what I know in case they're helpful in some way. I'll try to look at the code and comment better.

318anushka · 2020-04-07T09:57:01Z

Uploader is the username, right?

318anushka · 2020-04-07T10:00:08Z

I couldn't understand what you mean by this. Can you clarify?

See this image for example:

Author here is the custom name that i chose and that's what we want in Uploaded by i guess
Currently, author name is set as creator of contribution so that value is displayed everywhere. if we wish to change that then
.append("|author=[[User:").append(creator).append("|").append(creator).append("]]\n");
here instead of setting both as creator (which by default is username else author name) first field should be user.

318anushka · 2020-04-07T15:12:51Z

Okay i think i get it now. Few more things to clear before writing the code.

We want to display username or author in Uploaded by
Instead of new user page for every author we want to link it to the registered user
This issue still exists if user hasn't created a page

sivaraam · 2020-04-07T19:37:34Z

We want to display username or author in Uploaded by

If I were to just go by the phrase "Uploaded by" I would say it's best to show the uploader (the username of the person who uploaded it). To back up my conclusion let me quote a recent comment by @nicolas-raoul about the use case for the "Uploaded via Mobile" tab.

Context: "Uploaded via mobile" is meant to emulate good behavior by showing good pictures uploaded by others. It also allows experienced users to spot bad pictures and delete them, on the go.

The experienced users would expect the name of the uploader to be shown there as it would be helpful for them for their work. So, I think showing the uploader would be the appropriate thing to do.

Instead of new user page for every author we want to link it to the registered user

I don't get this. What do you mean by this and how does this relate to the issue?

This issue still exists if user hasn't created a page

I couldn't get this too.

I took a look at the code and it looks like we really are just showing the author name as "Uploaded by". This is misleading, IMO.

As an aside, it looks like the code that parses the author name would just return an empty string for some valid cases. For example: File:Troy Hunt.jpg. These are also the images for which you see no "Uploaded By:" line in the Explore, Uploaded via Mobile screens!
Also, it just seems to show creepy text in other cases. Example: File:Jean-François de Troy - A Hunting Meal - WGA23084.jpg
But that's not our concern now. We can look into those separately.

To conclude, I think we can fix this issue by properly showing uploader name[1] for "Uploaded by: ". This shouldn't be a big deal as we'll be showing the author name in the media detail view. Anyways, you can ask in the corresponding issue to see what the consensus is.

[1]: obtained from user parameter in imageinfo result

318anushka · 2020-04-07T19:53:19Z

I don't get this. What do you mean by this and how does this relate to the issue?

For new users who haven't created a user page. Like i uploaded via app without creating a page for a while and in Uploaded via mobile it displayed "page does not exist" next to my username.
Anyway, this is just to explain what I meant. your proposed solution shouldn't cause that problem. I'll check and let you know :-)

macgills · 2020-04-08T07:17:04Z

@318anushka this is my own ignorance showing here: We have to create a user page? Is a user page not created when you create an account? Surely you can't upload without an account?

318anushka · 2020-04-08T08:32:36Z

@318anushka this is my own ignorance showing here: We have to create a user page? Is a user page not created when you create an account? Surely you can't upload without an account?

yes, you have to create user page. Even if a user registers, page is not created on it own.

318anushka · 2020-04-08T10:44:50Z

@sivaraam No changes in media detail view? It'd still show the author name.
This is fixed

This still exists

I suggest creating a separate issue for this as it is related to custom author name.

neslihanturan · 2020-04-08T12:31:06Z

@318anushka this is my own ignorance showing here: We have to create a user page? Is a user page not created when you create an account? Surely you can't upload without an account?

Actually no, you don't need to have a userpage to be able to upload. If you are a registered user and you didn't created your userpage as https://commons.wikimedia.org/wiki/User:USERNAMEHERE this user is still exist, just the page describe themselves is not there. (I have read the long conversation very fast, please ignore me if I am saying something you already knew)

sivaraam · 2020-04-08T18:33:30Z

This is fixed

Done.

This still exists

I think we should fix that too. The following modified version of the Media#getArtist should do the trick:

    private static String getArtist(ExtMetadata metadata) {
        try {
            final String anchorStartTagTerminalChars = "\">";
            final String anchorCloseTag = "</a>";

            String artistHtml = metadata.artist();
            return artistHtml
                    .substring(
                        artistHtml.indexOf(anchorStartTagTerminalChars),
                        artistHtml.indexOf(anchorCloseTag)
                    )
                    .substring(anchorStartTagTerminalChars.length());
        } catch (Exception ex) {
            return "";
        }
    }

I hope this doesn't break anything, though.

I suggest creating a separate issue for this as it is related to custom author name.

As I've said before this isn't related to custom author name at all. The custom author name is just one of the ways that you could easily use to make the issue surface. It happens in general whenever a author of an image is a commons user who hasn't created a user page. Example: File:Sticker Domitilla Tete Pierre.jpg - Wikimedia Commons. This image was not uploaded using the app at all. But if you view this image using the app (by searching in Explore) or check the corresponding API call you should see the (page does not exist) part appended to the Author.

codecov-io · 2020-04-08T20:28:51Z

Codecov Report

Merging #3620 into master will increase coverage by 0.02%.
The diff coverage is 6.66%.

@@             Coverage Diff             @@
##             master   #3620      +/-   ##
===========================================
+ Coverage      6.95%   6.97%   +0.02%     
- Complexity      291     292       +1     
===========================================
  Files           256     256              
  Lines         11730   11749      +19     
  Branches        959     960       +1     
===========================================
+ Hits            816     820       +4     
- Misses        10843   10858      +15     
  Partials         71      71

Impacted Files	Coverage Δ	Complexity Δ
.../fr/free/nrw/commons/category/GridViewAdapter.java	`0.00% <0.00%> (ø)`	`0.00 <0.00> (ø)`
...r/free/nrw/commons/contributions/Contribution.java	`0.77% <0.00%> (ø)`	`1.00 <0.00> (ø)`
.../commons/contributions/ContributionsPresenter.java	`21.25% <0.00%> (ø)`	`5.00 <0.00> (ø)`
...w/commons/explore/images/SearchImagesRenderer.java	`0.00% <0.00%> (ø)`	`0.00 <0.00> (ø)`
...fr/free/nrw/commons/mwapi/OkHttpJsonApiClient.java	`0.00% <0.00%> (ø)`	`0.00 <0.00> (ø)`
...n/java/fr/free/nrw/commons/upload/UploadModel.java	`0.00% <0.00%> (ø)`	`0.00 <0.00> (ø)`
app/src/main/java/fr/free/nrw/commons/Media.java	`16.66% <15.38%> (-0.30%)`	`5.00 <0.00> (ø)`
...main/java/fr/free/nrw/commons/WelcomeActivity.java	`0.00% <0.00%> (ø)`	`0.00% <0.00%> (ø%)`
...n/java/fr/free/nrw/commons/CommonsApplication.java	`1.09% <0.00%> (ø)`	`1.00% <0.00%> (ø%)`
...fr/free/nrw/commons/media/MediaDetailFragment.java	`0.00% <0.00%> (ø)`	`0.00% <0.00%> (ø%)`
... and 1 more

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update eb88e58...6046c45. Read the comment docs.

macgills · 2020-04-09T07:48:02Z

app/src/main/java/fr/free/nrw/commons/Media.java

@@ -117,7 +120,7 @@ public static Media from(MwQueryPage page) {
        ExtMetadata metadata = imageInfo.getMetadata();
        if (metadata == null) {
            Media media = new Media(null, imageInfo.getOriginalUrl(),
-                    page.title(), "", 0, null, null, null);
+                    page.title(), "", 0, null, null, null, null);


ImageInfo doesn't have a user if metaData is null?

app/src/main/java/fr/free/nrw/commons/Media.java

macgills · 2020-04-09T07:54:48Z

app/src/main/java/fr/free/nrw/commons/category/GridViewAdapter.java

@@ -98,9 +98,9 @@ public View getView(int position, View convertView, ViewGroup parent) {
     * @param author
     */
    private void setAuthorView(Media item, TextView author) {


Is this still even an author? Uploader is probably more accurate?

macgills · 2020-04-09T07:59:20Z

app/src/main/java/fr/free/nrw/commons/contributions/ContributionsPresenter.java

@@ -107,7 +107,7 @@ public void fetchContributions() {
                    .filter(mwQueryLogEvent -> !mwQueryLogEvent.isDeleted()).doOnNext(mwQueryLogEvent -> Timber.d("Image %s passed filters", mwQueryLogEvent.title()))
                    .map(image -> {
                        Contribution contribution = new Contribution(null, null, image.title(),
-                                "", -1, image.date(), image.date(), user,
+                                "", -1, image.date(), image.date(), user, user,


this seems wrong in light of the other constructor where you set creator/user to different values
null, null, sessionManager.getAuthorName(), sessionManager.getUserName(),

This is for fetching current user's contributions and user value is username only

Okay so when we are fetching current users contributions

creator == sessionManager.getUserName() user == sessionManager.getUserName()

but when we are creating them to upload

creator == sessionManager.getAuthorName() user == sessionManager.getUserName()

That is correct behaviour?

Hmm, didn't notice that. we are not displaying author in media detail. don't know why it is set to username.
@sivaraam shouldn't creator be getAuthorName() in first one too.

@sivaraam any insight?

I am not sure if creator is a field we ever need right now at this point.

To add to what @318anushka said, creator is also useful as some images might literally have no relation between the Author and the person who uploaded it. Here's an example: File: Troy Hunt. The Author is Troy Hunt while the user who uploaded it is IagoQnsi.

This most likely comes into picture only in Explore, though. It should actually be a concern in Contributions too but let's give ourselves some break and not get into that for now ;)

When we create a Contribution for upload we set the creator to sessionManager.getAuthorName() aka just the user name

The author name information should come from the media rather than from the session manager but I believe we can't do it right now as we are using the logevents API. #2904 would change this, I believe.

We then upload the creator twice??

Just to clear the confusion, that's just the code for constructing the following wiki text:

[[User:USER_NAME | USER_NAME]]

The redundant USER_NAME is just to hide the User: prefix from the rendered wiki text.

It probably should be, I am now quite interested in what this artist metadata returns when we actually put the right data into this pageContents ...

I really couldn't get this. I think this PR is in a right shape now. If there's something that's still not clear let me know. I'll see if I could help with clarifying that.

Also while uploading
.append("|author=[[User:").append(contribution.getCreator()).append("|") .append(contribution.getCreator()).append("]]\n");
shouldn't first one of these field be username and then creator. I don't know about this just inferring from here

I have zero knowledge of the rationale behind custom author name. But I'm suspecting it's to enable users to have a custom name in author field (as opposed to allowing them to attribute images created by others) in which case you're right. The first one should be username. Anyways, that should probably be fixed separately.

This author/creator/artist/user stew is thoroughly annoying

Yeah. This is the mess you get when the data model of the app and that returned by the server don't align well. 🤷‍♂️

I am not particularly happy with this PR. Why did we bother extracting the contents of the anchor tag, removing the page does not exist suffix only to add and use a new different field?
Why is DeleteHelper still trying to remove page does not exist?

This new field gets assigned like sessionManager.getAuthorName(), sessionManager.getUserName(), and having these 2 identical damned methods beside each other adds more to confusion making them seem like separate things.

I think adding user is a mistake and perhaps the only changes in this PR should be getArtist and get rid of the suffix removal in DeleteHelper.

Viewed in browser the author field is, I believe, whatever we sent in author=[[User:myUserName|userName again?]]

And we should not differ from the website

Though I guess uploadedBy is information that isn't explicitly mentioned anywhere on the website? I'll seek confirmation on the ticket

I am not particularly happy with this PR. Why did we bother extracting the contents of the anchor tag, removing the page does not exist suffix only to add and use a new different field?

Because if we don't then the (page does not exist) would still show up in the detail view of the images you view via 'Explore'. The new field is to just show the username near the "Uploaded by:". We still show the "Author" in the media detail view.

Why is DeleteHelper still trying to remove page does not exist?

Thanks for bringing that up! We can remove that code now. cc @318anushka.
It really should've been caught during the PR that added that code. Looks like it slipped through.

Also, that's another place where the user would be the appropriate thing to use instead of the author/creator. Something to be done in a separate PR :)

This new field gets assigned like sessionManager.getAuthorName(), sessionManager.getUserName(), and having these 2 identical damned methods beside each other adds more to confusion making them seem like separate things.

Yeah. I think we can just go with user, user for that part instead of sessionManager.getAuthorName(), user. There's not much difference as of now like you point out.

I think adding user is a mistake and perhaps the only changes in this PR should be getArtist and get rid of the suffix removal in DeleteHelper.

I would say it's a "boon" for the reasons I mention above. You'll likely get more info about this in the issue thread where you've asked Nicolas for clarification. Let me know if you're still find some mistakes with introducing user. I'll see if I could clarify :)

Viewed in browser the author field is, I believe, whatever we sent in author=[[User:myUserName|userName again?]]

And we should not differ from the website

Do you mean we shouldn't be doing any kind of extraction from anchor tag for the Author? If yes, then I agree. It's best if show it similar to how the website shows the Author. But it would be a more involved change and is out of scope of this PR.

The HTML fiddling has it's limitations such as the "Author" field not being shown in the detail view for some images. I believe this fiddling with the HTML came into picture as we needed a username that we could use as a simple string for use in other places.

one reason it's getting quite confusing is because we are having different names for same field. How about we keep only 2 user and author for now, replacing creator with author. All of this including changes you mentioned above could be done in separate pr after this.

Also i was just waiting for nicolas's input, thought to make changes after clarifying.

app/src/main/java/fr/free/nrw/commons/explore/images/SearchImagesRenderer.java

318anushka · 2020-04-09T09:41:31Z

Yeah okay!!

…

On Thu, Apr 9, 2020, 3:09 PM Seán Mac Gillicuddy ***@***.***> wrote: ***@***.**** commented on this pull request. ------------------------------ In app/src/main/java/fr/free/nrw/commons/Media.java <#3620 (comment)> : > String artistHtml = metadata.artist(); - return artistHtml.substring(artistHtml.indexOf("title=\""), artistHtml.indexOf("\">")) - .replace("title=\"User:", ""); + return artistHtml + .substring( + artistHtml.indexOf(anchorStartTagTerminalChars), + artistHtml.indexOf(anchorCloseTag) + ) + .substring(anchorStartTagTerminalChars.length()); Yeah keep it in java, I was just idly musing. Maybe add the length on to the index call I think that flows better This however I would like to see — You are receiving this because you were mentioned. Reply to this email directly, view it on GitHub <#3620 (comment)>, or unsubscribe <https://github.com/notifications/unsubscribe-auth/AK4T7LFERF6OT37ZNTM5Z5LRLWJWTANCNFSM4L4IJKTQ> .

neslihanturan · 2020-04-22T08:10:34Z

This discussion is too long here, but I am sure @sivaraam and @macgills are pointing the correct issues. Despite so much effort of everyone we can't let this PR go to sleep:D @318anushka can you fix conflicts, are there anything else needs to be done?

318anushka · 2020-04-22T09:12:27Z

Yeah, actually there is an ongoing discussion on what field to reflect in uploadedBy.
And looks like everyone agrees to @sivaraam points starting here

@sivaraam @macgills should i proceed with solving conflicts? I guess there are some more changes that needs to be done. could you please tell me what to do further.

macgills · 2020-04-22T10:29:47Z

It might be easier to start over than resolve the conflicts, there has been quite a large change with the merging of structured data

318anushka · 2020-04-22T13:31:36Z

Sounds better!! okay so current changes are fine, i'll fix the rest.
Closing this PR now.

page does not exist removed from username

d3b2d01

tests-checker bot suggested changes Apr 3, 2020

View reviewed changes

318anushka added 2 commits April 9, 2020 01:44

username displayed in Uploaded by

1ee05fd

username displayed in Uploaded by

f6c27c5

macgills reviewed Apr 9, 2020

View reviewed changes

app/src/main/java/fr/free/nrw/commons/Media.java Outdated Show resolved Hide resolved

macgills reviewed Apr 9, 2020

View reviewed changes

app/src/main/java/fr/free/nrw/commons/explore/images/SearchImagesRenderer.java Outdated Show resolved Hide resolved

username displayed in Uploaded by

6046c45

sivaraam mentioned this pull request Apr 21, 2020

"Page does not exist" near some usernames in Explore #3341

Closed

318anushka closed this Apr 22, 2020

page does not exist removed from username #3620

page does not exist removed from username #3620

Conversation

318anushka commented Apr 3, 2020

tests-checker bot left a comment

Choose a reason for hiding this comment

macgills commented Apr 6, 2020

318anushka commented Apr 6, 2020

macgills commented Apr 6, 2020

318anushka commented Apr 6, 2020

macgills commented Apr 6, 2020

318anushka commented Apr 6, 2020

macgills commented Apr 6, 2020

318anushka commented Apr 6, 2020 • edited Loading

318anushka commented Apr 6, 2020

sivaraam commented Apr 7, 2020 • edited Loading

318anushka commented Apr 7, 2020

sivaraam commented Apr 7, 2020

sivaraam commented Apr 7, 2020 • edited Loading

318anushka commented Apr 7, 2020

318anushka commented Apr 7, 2020 • edited Loading

318anushka commented Apr 7, 2020

sivaraam commented Apr 7, 2020

318anushka commented Apr 7, 2020 • edited Loading

macgills commented Apr 8, 2020

318anushka commented Apr 8, 2020 • edited Loading

318anushka commented Apr 8, 2020 • edited Loading

neslihanturan commented Apr 8, 2020

sivaraam commented Apr 8, 2020

codecov-io commented Apr 8, 2020 • edited Loading

Codecov Report

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

sivaraam Apr 16, 2020 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

318anushka commented Apr 9, 2020 via email

neslihanturan commented Apr 22, 2020

318anushka commented Apr 22, 2020

macgills commented Apr 22, 2020

318anushka commented Apr 22, 2020

318anushka commented Apr 6, 2020 •

edited

Loading

sivaraam commented Apr 7, 2020 •

edited

Loading

sivaraam commented Apr 7, 2020 •

edited

Loading

318anushka commented Apr 7, 2020 •

edited

Loading

318anushka commented Apr 7, 2020 •

edited

Loading

318anushka commented Apr 8, 2020 •

edited

Loading

318anushka commented Apr 8, 2020 •

edited

Loading

codecov-io commented Apr 8, 2020 •

edited

Loading

sivaraam Apr 16, 2020 •

edited

Loading