-
Notifications
You must be signed in to change notification settings - Fork 1.4k
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
Clean up remaining Google Drive projects #5830
Conversation
c643abb
to
3cdd5af
Compare
This pull request removes Google Drive functionality converting GD projects that couldn't be removed to ODK protocol so we need to update the banner: |
.../src/androidTest/java/org/odk/collect/android/feature/projects/GoogleDriveDeprecationTest.kt
Outdated
Show resolved
Hide resolved
if (result == DeleteProjectResult.UnsentInstances || result == DeleteProjectResult.RunningBackgroundJobs) { | ||
unprotectedSettings.save(ProjectKeys.KEY_PROTOCOL, ProjectKeys.PROTOCOL_SERVER) | ||
unprotectedSettings.save(ProjectKeys.KEY_SERVER_URL, "https://example.com") | ||
settingsProvider.getMetaSettings().save(MetaKeys.getKeyIsOldGDProject(it.uuid), true) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Why save this to meta settings and not just to normal ("unprotected") settings?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
My thinking was that unprotected settings represent the section of unprotected preferences in the app, the same with the protected ones and if we want to save something that does not have a visual representation then it should be in meta settings.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I'd more thought that anything that's a "project" level goes in "unprotected". I guess this is really information "about" the project itself rather than a setting so I do see why it's in meta, but it feels a little awkward that you have to still index by project. Would it be a possibility that this could live on Project
itself?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Would it be a possibility that this could live on Project itself?
Yeah, I think it's a good idea.
collect_app/src/main/java/org/odk/collect/android/instancemanagement/SubmitException.kt
Outdated
Show resolved
Hide resolved
2af5a37
to
514f869
Compare
514f869
to
ba96de2
Compare
Sorry for missing this comment! What about: "Your project uses Google Drive. This functionality will be removed X date." I think it would be great to say the exact date, rather than people having to figure out when it's going to happen. |
@alyblenkin this is one last informational message for people who still had submissions in their GDrive projects. "This project was previously connected to a Google Drive account. Google Drive support has been removed. You can configure a server or pull submissions to a computer." The last sentence could just be in the "learn more" but it seems somewhat helpful to give an idea of what needs to be done now. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I installed 2022.4.4 and created a GD project with a draft and a finalized form. I upgraded Collect there is no banner in the main menu. |
@grzesiek2010 Would you like me to file the issue above in a separate issue? |
@dbemke I think this would be the best approach - file new issues if you test pull requests that are already merged. |
Tested with Success! Verified on a device with Android 10 Verified cases:
|
Tested with Success! Verified on a device with Android 13 |
Closes #5403
Why is this the best possible solution? Were any other approaches considered?
The only thing of note here is that I've removed the protocol from server settings at all:
It's just one protocol now and we are not going add anything there soon I guess so it doesn't make sense to display it.
How does this change affect users? Describe intentional changes to behavior and behavior that could have accidentally been affected by code changes. In other words, what are the regression risks?
This removed GD integration completely. In one of the previous versions, we have added removing GD project that didn't have any unsent forms. Here we go one step further and if there are still projects like that we convert them to ODK protocol and get rid of the whole code responsible for working with GD.
That means we need to test that:
Do we need any specific form for testing your changes? If so, please attach one.
No.
Does this change require updates to documentation? If so, please file an issue here and include the link below.
Before submitting this PR, please make sure you have:
./gradlew checkAll
and confirmed all checks still pass OR confirm CircleCI build passes and run./gradlew connectedDebugAndroidTest
locally.