Skip to content
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

[Bug]: Editing depicts clears every other structured data associated with the image #5728

Open
sivaraam opened this issue May 13, 2024 · 14 comments · May be fixed by #5741
Open

[Bug]: Editing depicts clears every other structured data associated with the image #5728

sivaraam opened this issue May 13, 2024 · 14 comments · May be fixed by #5741

Comments

@sivaraam
Copy link
Member

Summary

Our editing of depicts seems to cause a bit more harm than good now. When trying to edit the depictions, the depictions are changed but all other structured data associated with the image such as the "caption", "creator" etc. are totally removed. We should make sure such a thing doesn't happen.

Sample edits that demonstrate this issue: [edit 1] [edit 2]

This one feels a bit serious and it is not even easy to revert such edits due to some abuse filter blocking such reverts [ref]. Can we just disable editing of depictions as a temporary measure until this one's fixed? If trivial, it would be great to just fix the core issue, of course 🙂

Steps to reproduce

  1. Login to the app
  2. Open an already uploaded image
  3. Try to edit the depiction for the image
  4. Check the edit history of that image on the website

Expected behaviour

Only the list of depictions of the structured data get modified as a consequence of the edit.

Actual behaviour

The edit results in all other structured data associated with the image being removed and only the new set of depicts being retained.

Device name

OnePlus Nord

Android version

12

Commons app version

5.0.1

Device logs

No response

Screen-shots

No response

Would you like to work on the issue?

None

@rohit9625
Copy link
Contributor

How long has this problem been here? Can I get any class reference to the related code?

@sivaraam
Copy link
Member Author

@sivaraam
Copy link
Member Author

The clear=1 in the linked line looks suspicious. It's worth exploring if toggling that helps.
Ref: https://www.wikidata.org/w/api.php?action=help&modules=wbeditentity

@rohit9625
Copy link
Contributor

Yes, you're right. This request is being sent to the server when updating depicts:
image

As referenced, when the clear flag is set, i.e., 1 or true, it clears the entity which is shown here in the history:
image

After deleting the clear=1 the entity is not considered to be cleared rather it changes the entity:
image

However, the depictions are passed by the Commons App, are appended to the previous ones even one depiction was already there.
image

@sivaraam
Copy link
Member Author

After deleting the clear=1 the entity is not considered to be cleared rather it changes the entity:

Ah. That's an interesting observation.

However, the depictions are passed by the Commons App, are appended to the previous ones even one depiction was already there.

Got it. So, it seems like the clear=1 was there for a reason. But at least are the other structured data information retained when the clear is dropped? If so, we could explore other possible parameters / endpoints that'll help us ensure only a specific property is cleared.

@rohit9625
Copy link
Contributor

Yes, the other data information is retained. One more thing I noticed in the docs, i.e., the clear flag should be set like clear=true but in our codebase, it was clear=1. Well, it is the same thing in computers but does the API know that?

@sivaraam
Copy link
Member Author

Yes, the other data information is retained. One more thing I noticed in the docs, i.e., the clear flag should be set like clear=true but in our codebase, it was clear=1. Well, it is the same thing in computers but does the API know that?

I don't think we have to worry about that discrepancy since the clearing was happening without any doubt. The examples in wbeditentity API page showcase how the data could be tweaked to remove something. It would be worth exploring it to see of we could tweak the structure of the data parameter we send such only the depictions are cleared/updated with the new ones.

@rohit9625
Copy link
Contributor

We have to pass the whole structured data so that the entity gets updated with previous data as well as edited data. Currently, we are passing only depicted item's IDs, that's why only that gets updated and other data is cleared out due to a clear flag.

@rohit9625
Copy link
Contributor

@sivaraam
The bots keep adding the structured data even if it was completely deleted by our application. So, is this a feature or was it always like this?

@rohit9625
Copy link
Contributor

Look at here: updating categories

This updation doesn't delete the old structured data and the reason is, the previous data is appended rather than clearing old data and adding just the modified one.

Here, this function is responsible for updating categories and there is a parameter "wikiText" that is being passed and manipulated with the updated categories.
image

Afterward, this function is responsible for replacing the old wikiText(The text that contains the file's data) with the new updated one.
image

However, this is the wikiText that was first fetched from the server and there is nothing like depictions, why?
image

What are your thoughts about this?

@sivaraam
Copy link
Member Author

@sivaraam The bots keep adding the structured data even if it was completely deleted by our application. So, is this a feature or was it always like this?

Yeah. Those are friendly bots that run in Commons just doing their job. No issues about that :-)

@sivaraam
Copy link
Member Author

sivaraam commented May 21, 2024

What are your thoughts about this?

You are confusing category edit and depictions edit. Both are different and even use different APIs. Categories exist in commons. Depictions has to do with structured data. You could check this article for reference on Structured data on Commons.

Hope that clears your confusion.

@rohit9625
Copy link
Contributor

Hello Sivaraam,

I want to read about these terms(like mainsnak and snaktype) shown in the screenshot. I found another action that can help with the desired behaviour but I am unsure about the parameters.
image

@rohit9625
Copy link
Contributor

Hey @sivaraam, I solved the issue but, the whole task is being done on two phases:-

  1. Deleting all depictions
  2. Updating depictions with the new data.

See history

Although, after so much research and debugging the second way I can see is, set clear flag, it'll delete the whole entity and then send updated depictions along with other related data that was deleted.

I hope I was helpful :)

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging a pull request may close this issue.

3 participants
@sivaraam @rohit9625 and others