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

Enable background pushing of changes to the cloud as well as automated auto-push #5054

Merged
merged 9 commits into from
Feb 25, 2024

Conversation

nirvn
Copy link
Member

@nirvn nirvn commented Feb 25, 2024

First, a glorious image:

image

Now the description 😉

This PR improves the delta file wrapper (keeping track of changes done by users) by adding an awareness of a "pushing" state. When in that state, the delta file wrapper will continue collecting changes, but will keep them pending until we either:

  • successfully pushed the previous deltas, upon which the deltas collected during the "pushing" state are added to a new deltas "ID"; or
  • fail at pushing the previous deltas, upon which the deltas collected during the "pushing" state are added on top of the previous deltas.

This means that we have a fully functional QField while pushing, insuring that no data addition/editing/deletion is loss while communicating to the cloud server.

At this stage, this already fixes a known data loss scenario: ongoing tracking would have the changes made while pushing disappear after the deltas were pushed (because until now the code assumed that no activity would happen during the pushing).

Where it gets cool is that the improvements allow us to do background pushing of changes in an automated and regular interval, which QField now does if the user toggles that option on (see above screenshot).

The auto-push every 30 minutes setting is not on by default, and is a cloud project-level setting, allowing for users to turn this on for cloud project A but not make use of it for cloud project B.

@qfield-fairy
Copy link
Collaborator

qfield-fairy commented Feb 25, 2024

Copy link
Collaborator

@suricactus suricactus left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Nice and long requested functionality! Thanks!

Wondering about the UI, did you consider adding this in the QField settings screen? I don't think this is something you want to change every now and then and adding it where it is on the screenshot feel a bit overwhelming to me. Another option is to add a hamurger menu on the screenshoted screen, where one can choose on/off of this option.

Ideally it should be a slider similar to the way the "dim screen timeout" works.

src/core/deltafilewrapper.h Show resolved Hide resolved
src/core/deltafilewrapper.h Show resolved Hide resolved
src/core/deltafilewrapper.h Show resolved Hide resolved
src/core/deltafilewrapper.h Show resolved Hide resolved
src/qml/qgismobileapp.qml Show resolved Hide resolved
src/core/qfieldcloudprojectsmodel.cpp Outdated Show resolved Hide resolved
src/core/qfieldcloudprojectsmodel.cpp Outdated Show resolved Hide resolved
src/qml/QFieldCloudPopup.qml Outdated Show resolved Hide resolved
src/core/deltafilewrapper.cpp Outdated Show resolved Hide resolved
src/core/deltafilewrapper.cpp Outdated Show resolved Hide resolved
@nirvn
Copy link
Member Author

nirvn commented Feb 25, 2024

@suricactus , thanks for the fast review, I suspected you'd be interested by this one :)

Your comments have all been addressed. As for the UI, I've moved the toggle to sit below the 3 action buttons for now:

image

We can't move it in the QField settings as it is not a app-wide setting, it is bound to the current cloud project. And as for an hamburger menu, I feel at this stage it'd just be burying the feature. IMHO, I think it's worth keeping it exposed this way for now.

Copy link
Collaborator

@suricactus suricactus left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks for the fast addressing of the review. Yup, agreed on your UX comment. Now it is much nicer and less error prone for fat fingers. As these cloud project settings increase in number, we can introduce the hamburger menu.

Two small comments, all the rest looks very good, thanks!

@nirvn
Copy link
Member Author

nirvn commented Feb 25, 2024

@suricactus , done.

@nirvn nirvn merged commit c85ecca into master Feb 25, 2024
25 checks passed
@nirvn nirvn deleted the background_push branch February 25, 2024 12:55
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants