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

Add further media types to auto upload #797

Merged
merged 208 commits into from
Aug 8, 2017
Merged

Conversation

AndyScherzinger
Copy link
Member

@AndyScherzinger AndyScherzinger commented Apr 3, 2017

as discussed in #285 and numerous other issues, this PR adds further media types to the auto upload screen, starting with videos. For now video and images are split and shown twice for folders contain both e.g. the camera folder. We could also merge them into one (if we want that which is subject to discussion).

Audio is also on the agenda but I would leave it out for now until it is clear how to implement this. The reason for that is that we are querying the media store and then display all folders that contain images (and are visible to the media store, as-in indexed). So for music in case each album has it's won folder we would display all those folders which might be a lot while it does makes sense do handle it this way for video and images...

here is a screenshot of the work in progress and btw thumbnail generation and caching has been extended for video. If a format can't be read or the file is somehow corrupt we just show the standard icon:
device-2017-06-09-172344

OPEN ISSUES

  • add delete capability for custom folders (delete button on the bottom left in the folder preference dialog)
  • hide delete for non custom folders
  • hide delete for unpersisted custom folders
  • block save button in preference screen as long as not all necessary data has been set (grey out "save" text until set)
  • fix filter for local folder picker (upload activity re-use) to only show folders but no files
  • make Jobs aware of the folder type and only upload all file types for custom folders, video-only for video folders and image-only for image folders
  • make sure a job gets removed/terminated when a custom folder setup gets deactivated or deleted
  • rewrite auto upload
  • adapt to customer-hack-flag which hides the per-folder setting, which is needed for custom folders and also the flag needs to be ignored for custom folders since the user (IMHO) will like to have full control over this type of auto upload folder!
  • add check to prevent several custom folder definitions which define syncs on different levels of one file tree since this will either lead to duplicate uploads or even errors (in case delete after upload)
  • check why successful uploads disappear from uploads view after visiting that screen a few times
  • lock files while uploading
  • show only current user uploads
  • automatically retry manual uploads
  • remove uploads screen menu
  • show "Setup custom folder" even if no media folders are detected
  • Make it "Set up a custom folder" where it says "Setup a custom folder" on the work in progress screen

@AndyScherzinger AndyScherzinger added 2. developing design enhancement needs info Waiting for info from user(s). Issues with this label will auto-stale. labels Apr 3, 2017
@mario
Copy link
Contributor

mario commented Apr 3, 2017

Hi @AndyScherzinger. First of all nice work.

a) Any specific questions?
b) No need to really I think
c) Sure as we might want to operate on that later on - integer please (0, 1, 2, etc)

@AndyScherzinger
Copy link
Member Author

Thanks for the feedback @mario than I know how to proceed.

  1. Will keep video and images separately
  2. Will extend the db model to save the folder type

After that maybe one of you two can then work with the folder type for images and video so no videos get uploaded for an image folder and vice versa.
After that I'll then add custom folders but probably in a new PR.

Fine with you @mario @tobiasKaminsky?

@mario
Copy link
Contributor

mario commented Apr 3, 2017

@AndyScherzinger I'm unsure if that's desirable - I would just upload whatever ends up being in the folder. But you tell me what you prefer cc @tobiasKaminsky

Update: ignore me, is too late xD

@AndyScherzinger
Copy link
Member Author

I am also fine either way it is just that people asked for the separation of images and video. If we don't want to do this then I need to merge folders like camera, since it shows up twice since it is found in both media stores

@mario
Copy link
Contributor

mario commented Apr 3, 2017

Yea, I was silly - just ignore me as I said :D Is late and I have a headache.

@AndyScherzinger
Copy link
Member Author

AndyScherzinger commented Apr 3, 2017

So not an issue we just need to decide it. If we split it than it'll work like before since probably only camera folders put both into one directory..

@mario
Copy link
Contributor

mario commented Apr 3, 2017

Like in your screenshot seems fine.

@tobiasKaminsky
Copy link
Member

The only drawback I currently see is, that an user (like m) who wants to have video and images uploaded in the same directory, both on wifi only, have to set it up twice...
So I still would go for an approach like I mentioned here #486 (comment) but this is also net perfect, especially for the space constraints @AndyScherzinger mentioned.

I fear that this will only satisfy few users and the rest is still "complaining". So we should (at least plan) a broader approach as now audio and non-media-files are missing, and we do not have a file sized rule.

@mario
Copy link
Contributor

mario commented Apr 4, 2017

I would maybe ignore audio anyway, and just go for "Photos", "Videos" and "Others" type, where custom folders would only have "Others", and upload everything.

@AndyScherzinger AndyScherzinger force-pushed the mediaStoreAutoUpload branch 2 times, most recently from 77e2e09 to 3ba165a Compare April 5, 2017 09:04
@AndyScherzinger AndyScherzinger force-pushed the mediaStoreAutoUpload branch 3 times, most recently from 5a19877 to c0d7102 Compare April 19, 2017 07:55
@AndyScherzinger
Copy link
Member Author

@tobiasKaminsky This is the PR we were talking about which might (at some point) implement the custom folders depending on my time (or anyone else`s)

@AndyScherzinger AndyScherzinger force-pushed the mediaStoreAutoUpload branch 8 times, most recently from be53dea to 913c3fc Compare April 25, 2017 08:28
@AndyScherzinger AndyScherzinger force-pushed the mediaStoreAutoUpload branch 2 times, most recently from 0d11844 to e0f5913 Compare April 27, 2017 08:03
@mario
Copy link
Contributor

mario commented Apr 27, 2017

@AndyScherzinger so ... list of tasks? :D

@AndyScherzinger
Copy link
Member Author

AndyScherzinger commented Aug 8, 2017

👍 code fine by me, latest CR fixes done myself and also tested the update routine for the DB

  • 188 comment
  • 206 commit
  • 314 files changed

@tobiasKaminsky awaiting your final approval :)

Approved with PullApprove

Approved with PullApprove

@AndyScherzinger
Copy link
Member Author

👍 code fine by me, latest CR fixes done myself and also tested the update routine for the DB

  • 191 comment
  • 203 commit
  • 309 files changed

@mario awaiting your final approval :)

@mario
Copy link
Contributor

mario commented Aug 8, 2017

👍

Approved with PullApprove

@AndyScherzinger
Copy link
Member Author

@mario @tobiasKaminsky please wait until drone completes before hitting merge! Having this PR with all checks turned green would be awesome!!!

@AndyScherzinger
Copy link
Member Author

@mario @tobiasKaminsky nevermind :/ Tobias' lint fixes will trigger a bot push and thus another change rendering all approval obsolete... So whenever drone is done with the 2 rounds, let's merge... ;)

@mario
Copy link
Contributor

mario commented Aug 8, 2017

@AndyScherzinger so you will merge that other PR in here?

@AndyScherzinger
Copy link
Member Author

No, I'll merge now!

Cc @tobiasKaminsky

@AndyScherzinger AndyScherzinger merged commit bfe2b71 into master Aug 8, 2017
@AndyScherzinger AndyScherzinger deleted the mediaStoreAutoUpload branch August 8, 2017 11:48
@tobiasKaminsky
Copy link
Member

Hurray 🎉 🎉 🎉 🎉 🎉 🎉

@oparoz
Copy link
Member

oparoz commented Aug 8, 2017

Many thanks guys!

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.

9 participants