Apps-android-commons: Merge Structured Data branch into master
- Clone this repository somewhere on your computer: https://github.com/VitalyVPinchuk/apps-android-commons/tree/upload_caption_depict
- Switch to the upload_caption_depict branch
- Test it: Browse by items, see parent and child items
- Test it more: Upload a picture via Nearby and post below the URL of the uploaded picture
- Merge the latest master of
[email protected]:commons-app/apps-android-commons.git - Fix the conflicts
- Test again (steps 3 and 4)
- Let us know when you are done by posting a comment below
nicolas-raoul
All 58 comments
- Clone this repository somewhere on your computer: https://github.com/VitalyVPinchuk/apps-android-commons/tree/upload_caption_depict
- Switch to the upload_caption_depict branch
- Test it: Browse by items, see parent and child items
- Test it more: Upload a picture via Nearby and post below the URL of the uploaded picture
- Merge the latest master of
[email protected]:commons-app/apps-android-commons.git- Fix the conflicts
- Test again (steps 3 and 4)
- Send pull request to
[email protected]:commons-app/apps-android-commons.git
Can I work on this issue?
GearGit
on 25 Nov 2019
@GearGit : Yes, thanks! Please let us know about your progress at least once per day :-)
nicolas-raoul
on 25 Nov 2019
@GearGit : Yes, thanks! Please let us know about your progress at least once per day :-)
I will surely update my progress daily, as a matter of fact, I have already started working on it.
GearGit
on 25 Nov 2019
@nicolas-raoul Can I use Travis CI for merging conflicts?
GearGit
on 26 Nov 2019
Not sure how it might help, but feel free to try.
On Tue, 26 Nov 2019, 13:33 Somanshu, notifications@github.com wrote:
@nicolas-raoul https://github.com/nicolas-raoul Can I use Travis CI for
merging conflicts?—
You are receiving this because you were mentioned.
Reply to this email directly, view it on GitHub
https://github.com/commons-app/apps-android-commons/issues/3222?email_source=notifications&email_token=AAAYKBWCFV22ZHXRTFHWZ7DQVSRIVA5CNFSM4JRCNLAKYY3PNVWWK3TUL52HS4DFVREXG43VMVBW63LNMVXHJKTDN5WW2ZLOORPWSZGOEFEVTHI#issuecomment-558455197,
or unsubscribe
https://github.com/notifications/unsubscribe-auth/AAAYKBTMX7JTJTGNSRYYZNTQVSRIVANCNFSM4JRCNLAA
.
nicolas-raoul
on 26 Nov 2019
I am not sure if its a beginner-friendly issue. Apart from resolving the merge conflicts, this task would also require fixing the Wikidata API calls. None of the authenticated API were working earlier.
maskaravivek
on 26 Nov 2019
Vivek, thanks for the insight!
I think the merging is newcomer-friendly.
Then URLs can be replaced by an expert 🙂
On Tue, 26 Nov 2019, 13:51 Vivek Maskara, notifications@github.com wrote:
I am not sure if its a beginner-friendly issue. Apart from resolving the
merge conflicts, this task would also require fixing the Wikidata API
calls. None of the authenticated API were working earlier.—
You are receiving this because you were mentioned.
Reply to this email directly, view it on GitHub
https://github.com/commons-app/apps-android-commons/issues/3222?email_source=notifications&email_token=AAAYKBX36CAOCRRSKSAERDLQVSTONA5CNFSM4JRCNLAKYY3PNVWWK3TUL52HS4DFVREXG43VMVBW63LNMVXHJKTDN5WW2ZLOORPWSZGOEFEWQ6A#issuecomment-558459000,
or unsubscribe
https://github.com/notifications/unsubscribe-auth/AAAYKBWQFXJNZHBRSCNV4JDQVSTONANCNFSM4JRCNLAA
.
nicolas-raoul
on 26 Nov 2019
I think the merging is newcomer-friendly.
Are we talking about merging master into the structured-data branch? If yes, then it should be fine that anyone picks it. :)
Else, if we are talking about merging structured-data into master then everything will need to work perfectly before it can be merged.
Then URLs can be replaced by an expert 🙂
Also, the URLs are correct AFAIK, but some changes need to be done in how the edit token is fetched. Also, some of the APIs need to be modified to send multipart request params instead of application/x-www-form-urlencoded.
maskaravivek
on 26 Nov 2019
Are we talking about merging master into the structured-data branch?
Yes, that is an important task to ensure it stays up-to-date.
Else, if we are talking about merging structured-data into master then everything will need to work perfectly before it can be merged.
Also, the URLs are correct AFAIK, but some changes need to be done in how the edit token is fetched. Also, some of the APIs need to be modified to sendmultipartrequest params instead ofapplication/x-www-form-urlencoded.
Is there an issue about this? If not it would be great to create one, so that we don't forget some todo or tips :-)
nicolas-raoul
on 26 Nov 2019
Yes, that is an important task to ensure it stays up-to-date.
Do you mind updating the task title and description?
Is there an issue about this? If not it would be great to create one, so that we don't forget some todo or tips :-)
Sure, will do. :)
maskaravivek
on 26 Nov 2019
Fixed, thanks! :-)
nicolas-raoul
on 26 Nov 2019
@nicolas-raoul Do I have to still merge the https://github.com/VitalyVPinchuk/apps-android-commons/tree/upload_caption_depict
Branch with the master branch?
If yes,then who will be handling the URLs.
GearGit
on 26 Nov 2019
@GearGit Yes please clone https://github.com/VitalyVPinchuk/apps-android-commons/tree/upload_caption_depict then merge the main repo's master into it.
The URLs/etc will be handled by someone else (or even you if you understand everything and are motivated) in the issue Vivek will create.
nicolas-raoul
on 26 Nov 2019
@nicolas-raoul I have cloned the branch into my local machine and tried merging it with the master branch, but there are several conflicts which I have to manually correct.
Should I simply just allow the new changes to overwrite the old ones or do I also have to check how is it affecting the master branch?
Test it: Browse by items, see parent and child items
Also, I didnt quite get this part, therefore it would be very helpful if you can provide a screenshot or some information.
GearGit
on 27 Nov 2019
Should I simply just allow the new changes to overwrite the old ones or do I also have to check how is it affecting the master branch?
That is the difficulty or merging conflicts: You have to understand the code, and keep the features and improvements of both sides. Good luck! :-)
Test it: Browse by items, see parent and child items
Before merging, run the app and go to the "Explore" section then use the search bar and tap everything that talks about items. Also make sure to try and understand how this page works (when uploading a picture):
nicolas-raoul
on 27 Nov 2019
@nicolas-raoul @maskaravivek I found this bug when I was searching for a bow in the explore section, don't know the exact reason for this bug.
Whenever I am searching for some caption under items -> media tab, "No images found" message is displayed for any caption that I enter, but if I enter same caption under the media tag then it works fine.
Furthermore, when I am checking the parent and child classes, then I am sometimes encountering the following leak:
Due to this error, the app sometimes crashes, and in the leaks app, there are multiple leaks displayed like ReportFragment leaked, WikidataItem-DetailsActivity leaked and many more.
How should I go about fixing this?
GearGit
on 29 Nov 2019
@GearGit thanks for reporting this. Please create a separate issue for this and any other issues that you encounter on this branch.
maskaravivek
on 29 Nov 2019
@GearGit Great work! As Vivek said, please create issues for each problem (be sure to mention what repo+branch in each issue), and then for each issue we can guide you on how to fix it :-)
nicolas-raoul
on 29 Nov 2019
@nicolas-raoul @maskaravivek Take another example, when I type flowers.
,
But in the play store version of app I am able to fetch the data without any issue.
Maybe I should open seperate issue for this.
GearGit
on 30 Nov 2019
Yes please! Don’t hesitate in creating new issues even if you are not sure that it deserves a separate issue or not :)
maskaravivek
on 30 Nov 2019
@maskaravivek @nicolas-raoul Should I create a new issue here or in the repository https://github.com/VitalyVPinchuk/apps-android-commons/tree/upload_caption_depict
GearGit
on 30 Nov 2019
Here. Thanks 🙂
On Sat, 30 Nov 2019, 14:47 Somanshu, notifications@github.com wrote:
Should I create a new issue here or in the repository
https://github.com/VitalyVPinchuk/apps-android-commons/tree/upload_caption_depict—
You are receiving this because you were mentioned.
Reply to this email directly, view it on GitHub
https://github.com/commons-app/apps-android-commons/issues/3222?email_source=notifications&email_token=AAAYKBVCBCRFQPKM4IXUUELQWH463A5CNFSM4JRCNLAKYY3PNVWWK3TUL52HS4DFVREXG43VMVBW63LNMVXHJKTDN5WW2ZLOORPWSZGOEFP2ABI#issuecomment-559915013,
or unsubscribe
https://github.com/notifications/unsubscribe-auth/AAAYKBQPQKDCEXJEXUZSUCLQWH463ANCNFSM4JRCNLAA
.
nicolas-raoul
on 30 Nov 2019
@nicolas-raoul @maskaravivek i have made some changes in the categories and media section of explore in #3254
Please verify so that I can create a pull request and merge upload_caption_depict branch with master branch.
GearGit
on 7 Dec 2019
Sorry I have no time to check this month, anyone please check and let us
know, thanks 🙂
On Fri, 6 Dec 2019, 19:29 Somanshu, notifications@github.com wrote:
@nicolas-raoul https://github.com/nicolas-raoul @maskaravivek
https://github.com/maskaravivek i have made some changes in the
categories and media section of explore in #3254
https://github.com/commons-app/apps-android-commons/issues/3254
Please verify so that I can create a pull request and merge
upload_caption_depict branch with master branch.—
You are receiving this because you were mentioned.
Reply to this email directly, view it on GitHub
https://github.com/commons-app/apps-android-commons/issues/3222?email_source=notifications&email_token=AAAYKBXGVJF3TXXQQ4DXWITQXMKDPA5CNFSM4JRCNLAKYY3PNVWWK3TUL52HS4DFVREXG43VMVBW63LNMVXHJKTDN5WW2ZLOORPWSZGOEGF4U4Q#issuecomment-562809458,
or unsubscribe
https://github.com/notifications/unsubscribe-auth/AAAYKBUIF55BWY35WB2BSCTQXMKDPANCNFSM4JRCNLAA
.
nicolas-raoul
on 15 Dec 2019
@GearGit Does your branch https://github.com/GearGit/apps-android-commons/tree/upload_caption_depict compile and run correctly? Could you please post a screenshot of its "Explore" activity? Thanks :-)
nicolas-raoul
on 4 Jan 2020
Sorry for the delay in responding, but the https://github.com/GearGit/apps-android-commons/tree/upload_caption_depict branch does compile. I will upload a screenshot of the explore activity.
@GearGit Does your branch https://github.com/GearGit/apps-android-commons/tree/upload_caption_depict compile and run correctly? Could you please post a screenshot of its "Explore" activity? Thanks :-)
GearGit
on 13 Jan 2020
Looks like there is some issue with the upload_caption_depict branch, the "Explore" section of the app, as well as the "Uploaded via Mobile" both display the message "No images found!".
I will look into it and make a pull request.
@nicolas-raoul my previous pull request hasn't been yet merged by the collaborators. Is it maybe because I pulled the request on upload_caption_depict branch of https://github.com/VitalyVPinchuk/apps-android-commons
GearGit
on 13 Jan 2020
@nicolas-raoul my previous pull request hasn't been yet merged by the collaborators. Is it maybe because I pulled the request on upload_caption_depict branch of https://github.com/VitalyVPinchuk/apps-android-commons
I think we should make a copy of the upload_caption_depict branch to our repo and continue working on that. @nicolas-raoul Do you see any issues in doing that?
maskaravivek
on 17 Jan 2020
Sorry, I just looked at the discussion in detail. Also, can someone catch me up with details about why we are trying to update @VitalyVPinchuk's branch instead of @vanshikaarora's branch?
Isn't #2970 the original GSoC PR?
maskaravivek
on 17 Jan 2020
@nicolas-raoul @maskaravivek
The same issue persists with the '2.11-release' branch of the commons app. Is it because I am running the 2.11 release directly onto my phone from Android Studio or is it because of something else, cause I don't seem to be able to login either due to network error?
Even though I have a high-speed network, the network issue still persists, be it 'upload_caption_depict branch' or the '2.11-release' branch.
GearGit
on 19 Jan 2020
The login issue is been discussed in #3320. Yes, its specific to beta app.
maskaravivek
on 19 Jan 2020
@maskaravivek How should I go about with the app because the app cant seem to connect to the network, and also the login fails with error message : "Chain Authentication Failed"?
This seems to be common in the beta version on all branches, master,2.11-release and also in the upload_caption_depict.
I think I found the solution to #3327 .How can I switch to the production version of the app to resolve and test the issue and continue testing upload_caption_depict branch?
GearGit
on 20 Jan 2020
@GearGit to build and run the production flavor, you can change the build flavor to prodDebug or prodRelease. This is how you can change build flavor (I would very much recommend not to upload test pictures on prod server, your account might be blocked)
ashishkumar468
on 20 Jan 2020
@maskaravivek Vitaly took Vanshika's last commit and added several fixes, then last but not least, merged from master 2 months ago: https://github.com/VitalyVPinchuk/apps-android-commons/commit/53d160ba7612e0fe8a75451ea946275dbe8f71fd
nicolas-raoul
on 20 Jan 2020
@GearGit to build and run the production flavor, you can change the build flavor to prodDebug or prodRelease. This is how you can change build flavor (I would very much recommend not to upload test pictures on prod server, your account might be blocked)
What methodology would you recommend me to test the issue #3327 ? If beta servers are malfunctioning then this is a major concern and maybe we should open an independent issue for it.
@nicolas-raoul if the upload_caption_depict branch has been merged then maybe we should close this issue.
GearGit
on 21 Jan 2020
What methodology would you recommend me to test the issue #3327 ?
I answered you on the issue itself.
If beta servers are malfunctioning then this is a major concern and maybe we should open an independent issue for it.
Yes, I think this is being tracked at #3320
misaochan
on 21 Jan 2020
After merging the latest master into upload_caption_depict branch, some testing is needed, to make sure that at least pictures can be uploaded with crash :-) Then hopefully the upload_caption_depict branch can be merged into master, if @misaochan is OK (note: there are a few remaining bugs such as wrong thumbnails, I hope these are not grave enough to prevent merging into master?)
nicolas-raoul
on 21 Jan 2020
I agree with @nicolas-raoul. Once we have the basic sanity of the upload_caption_depict its best to merge it to master so that working on it becomes easier and we can create separate issues for remaining bugs that can be picked by volunteers.
maskaravivek
on 21 Jan 2020
Sorry for the delay in catching up on this thread. The reason I would prefer to wait is that we will likely have a developer (Sean from Kiwix) working on Structured Commons for us starting February. His first task would be getting Vanshika's work master-ready to create a foundation for him to work on. So if we can wait for him for a bit, it may be worth it?
Edit: Pinging @macgills for his input.
misaochan
on 21 Jan 2020
I don't have a tremendous amount of input right now as I don't have much of a grasp of the codebase or features yet. I'm happy for a piece of work to be left for me as long as it is not blocking any other task somebody wants to work on
macgills
on 22 Jan 2020
@macgills Ashish and I were mentoring Vanshika during her GSoC.
If you have any doubt on how the app is supposed to work with structured data, please ping me :-)
nicolas-raoul
on 22 Jan 2020
@misaochan what would be the acceptance criteria for this ticket?
After merging the latest master into upload_caption_depict branch, some testing is needed, to make sure that at least pictures can be uploaded with crash
I can verify you can upload a picture without crashing, tested on betaDebug. I did however get 2 items shown in the list, subsequently a crash when clicking the 2nd item but that bug was introduced in #3133.
To what extent must I verify this is working?
first task would be getting Vanshika's work master-ready to create a foundation for him to work on
The change is clocking in +7,253 −213 so I may need a definition of master ready or I could be doing this for a week or two.
macgills
on 2 Mar 2020
@nicolas-raoul do you have any guidance on the acceptance criteria of this ticket?
macgills
on 9 Mar 2020
@macgills Wow, sorry, I didn't get notified of this ping for some reason! Could you please link me to the image on Commons that you uploaded with this branch? The caption and depicts fields should be appropriately filled in the Commons file page.
subsequently a crash when clicking the 2nd item but that bug was introduced in #3133.
How did #3133 introduce a new crash?
Aside from that, here are the issues with the branch that I can see so far:
From @maskaravivek :
Apart from resolving the merge conflicts, this task would also require fixing the Wikidata API calls. None of the authenticated API were working earlier.
Also, the URLs are correct AFAIK, but some changes need to be done in how the edit token is fetched. Also, some of the APIs need to be modified to send multipart request params instead of application/x-www-form-urlencoded.
From @nicolas-raoul :
Test it: Browse (Explore) by items, see parent and child items
Test it more: Upload a picture via Nearby and post below the URL of the uploaded picture
Merge the latest master of [email protected]:commons-app/apps-android-commons.git
Fix the conflicts
Test again (steps 3 and 4)
Let us know when you are done by posting a comment below
From @GearGit :
Whenever I am searching for some caption under items -> media tab, "No images found" message is displayed for any caption that I enter, but if I enter same caption under the media tag then it works fine.
Furthermore, when I am checking the parent and child classes, then I am sometimes encountering the following leak:
Also, for an easy-to-read summary of what the PR is supposed to do, you can read this: https://medium.com/@vanshikaa937/google-summer-of-code19-final-post-d78e601ea39d . I think it's OK for this task to take you 1-2 weeks, if the final result is a polished implementation of depicts and captions in our upload process and Explore feature.
misaochan
on 9 Mar 2020
Thank you for the reply @misaochan , I will contact you promptly in the future.
How did #3133 introduce a new crash?
While the intentions were noble the result the code does not do the same thing, if the context is not CategoryImagesCallback then there is a ClassCast, code
I have uploaded a picture on beta but beta performs poorly with this branch so I have switched to using prod which functions better. I have however not been uploading to prod because I thought that would get me flagged, I believe someone mentioned that there is a list of images that an be safely uploaded to prod? If there is could I be linked to them?
You can use Explore and see items but the navigation has some errors in it when navigating deeply.
There will need to be a fair bit of rewriting I would think, Room should definitely be introduced and converting chunks to kotlin would certainly have great benefit.
As a plan for work where I am not silo-ed off I think we should revive this suggestion from @maskaravivek
I think we should make a copy of the upload_caption_depict branch to our repo and continue working on that
With the aim of me making tickets that can get reviewed as I merge them to the branch.
I would merge to master but the downside of most of these features being unusable on beta would add too much confusion to the project I think.
macgills
on 10 Mar 2020
While the intentions were noble the result the code does not do the same thing, if the context is not CategoryImagesCallback then there is a ClassCast, code
Ah I see, could you make an issue for this please? This will need to be fixed separately.
I have uploaded a picture on beta but beta performs poorly with this branch so I have switched to using prod which functions better.
Absolutely, I would recommend using prod as far as possible, unless you need to upload a lot of test photos.
I believe someone mentioned that there is a list of images that an be safely uploaded to prod? If there is could I be linked to them?
The easiest way is to spend 5 min outdoors just taking pictures of stuff - Commons needs all sorts of pictures, even if it's just a park or street or tree. :) But if this is a problem, I can email you some of my pictures if you like.
There will need to be a fair bit of rewriting I would think, Room should definitely be introduced and converting chunks to kotlin would certainly have great benefit.
I agree.
As a plan for work where I am not silo-ed off I think we should revive this suggestion from @maskaravivek : I think we should make a copy of the upload_caption_depict branch to our repo and continue working on that. With the aim of me making tickets that can get reviewed as I merge them to the branch.
This sounds like a good idea to me.
I would merge to master but the downside of most of these features being unusable on beta would add too much confusion to the project I think.
Yes, don't merge to master until these issues are fixed. A development branch would be perfect as you mentioned above.
misaochan
on 10 Mar 2020
Btw, just leaving a note here in case I forget later: It would be great if you could also make sure that the unit test coverage is up to par before we merge into master. It's possible to add the unit tests later, but it's easier to keep track of it if we just have them when the dev branch is merged.
misaochan
on 11 Mar 2020
Also, I guess #3484 would have to be taken care of so that on retrying failed uploads captions and depicts are not lost.
maskaravivek
on 11 Mar 2020
@macgills I have thousands of pictures to upload, here are some just for you, after you upload them all please please ask and I will add a hundred more :-)
nicolas-raoul
on 11 Mar 2020
Hi @nicolas-raoul , @ashishkumar468 was encountering difficulty with using your images due to not knowing the title and description that he should use to upload them. Could you add a brief one please? :)
misaochan
on 16 Mar 2020
@macgills @ashishkumar0207 When uploading my images, please use the title "Shibuya cityscape" or "Food in Shibuya restaurant" (feel free to adapt if obvious), and select the category Shibuya. Also, please tell me your Commons accounts by email or chat, so that I can see the pictures you uploaded and add a description and more precise categories. Thanks!
nicolas-raoul
on 16 Mar 2020
on prod I am macgills2
macgills
on 16 Mar 2020
Thanks @nicolas-raoul , my username is Ashishkumar468
ashishkumar468
on 16 Mar 2020
@misaochan
I would merge to master but the downside of most of these features being unusable on beta would add too much confusion to the project I think.
Yes, don't merge to master until these issues are fixed. A development branch would be perfect as you mentioned above.
So the issue with this is the beta environment, not necessarily the app (though a little bit the app).
Merging is becoming more crucial to stop divergences from arising so if we can live with it I would aim to get structured data merged sooner rather than later and live with the consequences.
macgills
on 20 Mar 2020
Yes, exactly. Due to the nature of how the beta cluster works, there are many features of the app that do not work on it. This is perfectly fine and intended (on our side) - if we restricted ourselves to an app that works perfectly on beta, we would have no Nearby uploads, no category suggestions, so on and so forth.
We should aim for it to work on production, and that is sufficient. Beta is solely to be used as a convenience tool for the developer (wherever possible), not as a target.
misaochan
on 20 Mar 2020
Okay, then I will open a PR to close this ticket
macgills
on 20 Mar 2020
Great! Please ping for testing when it's ready. :)
misaochan
on 20 Mar 2020
@misaochan ping
macgills
on 20 Mar 2020
Related issues
misaochan
·
4Comments
maskaravivek
·
3Comments
misaochan
·
4Comments
whym
·
3Comments
Saral-code
·
3Comments
Most helpful comment
@macgills I have thousands of pictures to upload, here are some just for you, after you upload them all please please ask and I will add a hundred more :-)