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

fix: Fallback to exporting preview-sized images if originals are missing #437

Merged
merged 3 commits into from
Oct 13, 2020

Conversation

gmaclennan
Copy link
Member

Contributor checklist:

  • My contribution is not related to translations. Please submit translation changes via our Mapeo Crowdin project
  • My commits are in nice logical chunks with good commit messages
  • I have walked through the QA Manual Testing
    Script
    and updated it if necessary.
  • If my changes depend upon an update to a dependency, I have updated the package-lock.json file using npm install --package-lock
  • My changes have been tested with the mobile app.
  • My changes are ready to be shipped to users on Windows, Mac, and Linux using the production version of the application built with electron-builder.

Description

It is possible for Mapeo Desktop to be missing originals of images attached to observations. This is not obvious to the user in the app because we always show the preview-sized images (1,200px). This can happen because phone-to-phone sync only syncs thumbnails and previews, not originals, to reduce disk space usage on phones. As a result if a phone has never synced with Desktop, only the previews and thumbnails will be available.

Currently, if the user tries to export observations that link to images with missing originals, via either the Webmaps export, or the data export with "Originals" selected, then the images will be missing from the exported data, with no warning shown to the user other than a file missing.txt included in the exported Zip. There is no way for the user to export a webmap with the images available (the preview-sized images).

This PR changes export behaviour to fallback to including preview-sized images if no originals are available. It changes the text files included in the Zip export to Export Errors.txt, which lists images that are not included in the export, and Missing Originals.txt which lists the images for which originals are not available.

The goal of this PR is to ensure that the user has a way to export and access the images that are available, even if the originals are not. The preview size (1,200px) is adequate for most online viewing and sending electronically.

The current method of feedback to the user about what has happened, via a text file included in the export, is not ideal, and at a later date we should add clearer feedback to the user with clear actions they can take to fix this.

For WebMap exports, it may make sense to default to preview-sized images to keep the webmap bundle size down anyway.

I have tested this by doing the following exports from the app with test data:

  1. Export webmap with all images present.
  2. Export webmap with missing originals but preview sizes present.
  3. Export webmap with missing images (originals and previews)
  4. Export data zip with "originals" selected, but originals missing (previews are included in export, Missing Originals.txt is in export with explanation).
  5. Export data zip with "previews" selected and all previews available (previews are included, as expected).
  6. Export data zip with "originals" selected, but missing images (no images in export, Export Errors.txt shows explanation).

@okdistribute
Copy link
Contributor

okdistribute commented Oct 7, 2020

This removes the missing.txt file - is there any documentation that references that file which needs to be changed?

Is this for 5.4 release?

@gmaclennan
Copy link
Member Author

This removes the missing.txt file - is there any documentation that references that file which needs to be changed?

I'm not aware of this being documented anywhere. I originally included this as a kind of "self documentation" for any user dealing with exported data, as a short-term step before we have a better UX in the app to show this kind of export issue.

Is this for 5.4 release?

I don't think it's urgent. It came up in a convo with Arky because workshop participants had been trying to use the "Export to webmap" functionality and it just didn't work. I felt like it's worth fixing since the webmaps functionality can be useful for some users, and since we have the service mainly working it's good to get this fixed.

@okdistribute
Copy link
Contributor

okdistribute commented Oct 8, 2020

@gmaclennan sorry for not being able to parse your response, but do you want this in 5.4.0 release?

@gmaclennan
Copy link
Member Author

Sorry about the lack of clarity. If this does not involve delaying the v5.4 release, then yes, include it. If including it would mean delaying the release, I think it is ok to leave until the next release, because it is not urgently needed.

@okdistribute
Copy link
Contributor

It looks good to me! We can get it in 5.4

@@ -5,62 +5,105 @@ import once from 'once'
import logger from '../logger'

const concurrency = 3
const errorsMsg =
'There was an error trying to export these files, so they are missing from this export:'
const missingOriginalsMsg =
Copy link
Contributor

Choose a reason for hiding this comment

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

This could be internationalized

Copy link
Member Author

Choose a reason for hiding this comment

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

Done

@okdistribute okdistribute merged commit 69c43ed into master Oct 13, 2020
@okdistribute okdistribute deleted the fix/export-missing-original-images branch October 13, 2020 19:14
@gmaclennan gmaclennan restored the fix/export-missing-original-images branch October 15, 2020 10:26
@gmaclennan gmaclennan deleted the fix/export-missing-original-images branch October 15, 2020 10:26
ErikSin pushed a commit that referenced this pull request Aug 3, 2021
* feat: auto-update Mapeo when internet is available (#346)

* chore(build): remove parallel requirement

* chore(release): 5.4.0-beta.0

* chore(build): Typo for uploading excecutables to AWS

* chore(release): 5.4.0-beta.1

* chore(release): Workaround attempt to fix an electron-builder bug

* chore(build): Add providers for github and s3 for each target

* chore(build): Manually add app-update.yml

* chore(release): 5.4.0-beta.2

* chore(build): try new app-update.yml through extraResources

* chore(release): 5.4.0-beta.3

* fix: Multiselect field should not error if has select_one data (#390)

* fix: Only allow updater when on compatible platforms

* fix: update to electron 9.1.2 (#389)

* chore(release): 5.4.0-beta.4

* fix: Use https://downloads.mapeo.app for updater endpoint

* fix: Error where download speed would always show 0 seconds

* fix: Only fetch download speed when opening the Updater tab.

* Revert "fix: Only fetch download speed when opening the Updater tab."

This reverts commit 6e5645f.

* fix: Properly return error when updater is inactive

* chore(release): 5.4.0-beta.5

* chore(release): 5.4.0-beta.5

* fix: Make left-hand side copy more simple

* fix: Ensure all dialogs will work with Electron 9 (#391)

* fix(typo): add missing 'notes' field in fallbackFields

* chore: Remove unnecessary matrix config from github action CD (#397)

* chore: Remove un-used architecture option for actions/setup-node (#394)

There is a PR to add an architecture option but it is not yet merged:
actions/setup-node#42

* fix: Notarize Mapeo for MacOS — fix warning / unable to open app (#396)

* fix: Notarize Mapeo for MacOS — fix warning / unable to open app

* fix: Add secrets

* fix: remove .map that should not have been there

* fix: Fix icon layout in dmg for MacOS (#399)

* chore: Put 'Check for Updates' in the help menu

* fix: Only save the language to config when changed manually. Fixes #400

Mapeo was saving the system locale to the config always on startup, so if you change system languages and reopened Mapeo it wouldn't also change system languages.

This now only persists the change in language if the user explicitly requests to change the language through the User Interface.

* fix: Mapeo config directory should be removable (#401)

* chore: Merge mapFilter@3.2.5 & add bundling & code-splitting (#393)

Co-authored-by: okdistribute <633012+okdistribute@users.noreply.github.com>

* New Report Generator (#303)

Co-authored-by: Gregor MacLennan <gmaclennan@digital-democracy.org>

* chore: package-lock

* chore(release): 5.4.0-beta.6

* fix: Ensure global OsmServerHost is always set

* fix: Mapeo should always open. Removes png editing for now.

* chore(lint): remove unused dependencies

* Update PULL_REQUEST_TEMPLATE.md

* chore: update translations

* chore(release): 5.4.0-beta.7

* fix: Cleanup report code

- Show toolbar during load
- Create ReportContentComponent -> allows memoizing PDFReport
- More robust loading code (patch for @react-pdf see
    diegomura/react-pdf#995)

* chore: gitignore generated worker files

* fix: Fix print button (still downloads, does not print)

PrintButton was using `this.url` instead of `this.props.url`.
However also removed some unnecessary code in this component

* fix: Fix map size (make square)

* fix: Render portrait images correctly and fix image width

* fix: Pull MAPBOX_ACCESS_TOKEN from a static location for both front
& backend

* feat: Add marker to map (first rough attempt)

* fix: make some progress on flow and types, not fixing all things but
many

* feat(staticMaps): Add concurrency and caching to speed up static maps

NB: Caching will not be noticable in dev, because currently npm start
and npm run dev will turn off http caching, but have tested this works
when http caching is enabled.

* feat(staticMaps): Support style and accessToken as URL query params

* feat(Reports): Use user defined map style for report maps

* fix(ReportMap): Add border around photos and simplify CSS

* fix(ReportDetails): Only show "Details" header when there are fields

* chore(lint)

* fix: translate report headers

* chore: translations

* fix: only display the first 5 character of the project secret key (#419)

* fix: only display the first 5 character of the project secret key

The rest is masked as "*"

* trigger GitHub actions

* fixed length mask to avoid potential UI scaling issues

* feat: display additional metadata in SyncFooter

* feat: show 'MAPEO' when there's no encryption key

* Add desktop architecture diagram

* chore(test): add proper mocks to storybook

* fix: Cleanup report code

- Show toolbar during load
- Create ReportContentComponent -> allows memoizing PDFReport
- More robust loading code (patch for @react-pdf see
    diegomura/react-pdf#995)

* chore: gitignore generated worker files

* fix: Fix print button (still downloads, does not print)

PrintButton was using `this.url` instead of `this.props.url`.
However also removed some unnecessary code in this component

* fix: Fix map size (make square)

* fix: Render portrait images correctly and fix image width

* fix: Pull MAPBOX_ACCESS_TOKEN from a static location for both front
& backend

* feat: Add marker to map (first rough attempt)

* fix: make some progress on flow and types, not fixing all things but
many

* feat(staticMaps): Add concurrency and caching to speed up static maps

NB: Caching will not be noticable in dev, because currently npm start
and npm run dev will turn off http caching, but have tested this works
when http caching is enabled.

* feat(staticMaps): Support style and accessToken as URL query params

* feat(Reports): Use user defined map style for report maps

* fix(ReportMap): Add border around photos and simplify CSS

* fix(ReportDetails): Only show "Details" header when there are fields

* chore(lint)

* fix: translate report headers

* chore: translations

* Update README.md

* fix: Ensure any stale processes are cleaned up before starting ipc workers

* fix: Save PDF now prints all observations

* fix: Refactor Save PDF to include typed options & fix flow

* chore(flow): Ensure datefilter value is defined when accessed

* fix: Styling of loading dialog & Add date/time report name

* fix: Map printer should work on the current offline style

* chore: Update desktop architecture documentation

* fix: write correct pid file

* chore: lint

* fix: Ensure mapeo always closes and show closing window while processes
are being destroyed. Fixes #398

* fix: wrap win/splash.close in try/catch as electron can throw

* fix: Show Single & Multi-select fields in Report View

* chore: Cleanup unused files

* chore(test): add proper mocks to storybook (#422)

* chore: move flow types into mapeo-schema (#420)

* fix: Bugs in menu

* Update QA.md

* fix: config now included in bundle

* fix: If subprocess failed with fatal error, it should not prevent Mapeo from closing.

* fix: Ensure process is always closed properly and mapeo can reopen easily in dev mode

* fix: Updater should not throw visible error when offline

* fix: standard

* fix: Sometimes Mapeo would continue running if Splash was closed before window finished loading

* fix: Leave port & map printer port control to Electron main process

* fix: if Mapeo fails to close gracefully, kill the process

* fix: Map printer will close when electron window closes it, no need to
wait for graceful close

* fix: cleanup debug messaging on killing node proc

* fix: ensure background code gets included in bundle

* fix: refactor node-ipc main process listeners

* fix: remember debugging settings on rotate and mapeo start

* fix: Splash screen would pause for a second with black background before starting animation

* chore(flow): update to latest mapeo-schema

* chore(deps): remove react-mapfilter, update package-lock.json

* fix: Use mapeo.ipc directly

* chore: Add debugger statements for errors when using menu

* chore: lint

* chore(flow): fix flow types

* fix: dont start heavier processes until splash screen is loaded

* fix: Multiselect fields with commas were not displaying correctly in Territory view

* chore(release): 5.4.0-beta.8

* fix: Prevent a bug that caused MapEditor to crash if you have no nodes after a sync.

* chore(release): 5.4.0-beta.9

* chore: Update CHANGELOG.md

* fix: Fix pdf report image wrapping (#436)

* fix: Extract English language messages for CrowdIn

* fix: Add separators in Help menu

* fix: Fallback to exporting preview-sized images if originals are missing (#437)

Co-authored-by: Karissa McKelvey <633012+okdistribute@users.noreply.github.com>

* Fix: Display error to user when it is a fatal mapeo core error (#439)

* New Crowdin updates (#388)

Co-authored-by: Karissa McKelvey <633012+okdistribute@users.noreply.github.com>

* chore: Add error log messages

* chore: Improve onboarding documentation for new devs.

* fix: Add Vietnamese, Thai, and Khmer to available languages.

* fix: Remove duplicate styling for updater loading progress.

* chore(release): 5.4.0-beta.10

* chore: lint

* chore: Add Thai translations for menus and main process strings (#445)

* New translations en.json (French)

* New translations en.json (Spanish)

* New translations en.json (Vietnamese)

* New translations en.json (Portuguese, Brazilian)

* New translations en.json (Khmer)

* New translations en.json (Thai)

* New translations en.json (Burmese)

* New translations en.json (Lao)

* New translations en.json (Thai)

* New translations en.json (Thai)

* New translations en.json (Thai)

* New translations en.json (Thai)

* New translations en.json (Thai)

* New translations en.json (Thai)

* New translations en.json (Thai)

* New translations en.json (Thai)

* New translations en.json (Thai)

* New translations en.json (Vietnamese)

* New translations en.json (Portuguese, Brazilian)

* New translations en.json (Spanish)

* chore: Remove untranslated strings (#450)

* New translations en.json (French)

* New translations en.json (Spanish)

* New translations en.json (Vietnamese)

* New translations en.json (Portuguese, Brazilian)

* New translations en.json (Khmer)

* New translations en.json (Thai)

* New translations en.json (Burmese)

* New translations en.json (Lao)

* New translations en.json (Thai)

* New translations en.json (French)

* New translations en.json (Spanish)

* New translations en.json (Vietnamese)

* New translations en.json (Portuguese, Brazilian)

* New translations en.json (Khmer)

* New translations en.json (Burmese)

* New translations en.json (Lao)

* feat: Add Thai translations (#451)

* New translations en.json (Thai)

* New translations en.json (Thai)

* New translations en.json (Thai)

* New translations en.json (Thai)

* New translations en.json (Thai)

* New translations en.json (Thai)

* New translations en.json (Thai)

* New translations en.json (Thai)

* New translations en.json (Thai)

* New translations en.json (Thai)

* New translations en.json (Thai)

* New translations en.json (Thai)

* fix: Ensure that fallback presets do not break iD Editor

iD Editor requires that presets `point`, `area`, `line`, `relation` are
always defined. The user can override these via mapeo config to give
them custom names and fields, however the user could also accidentally
set the `tags` and `geometry` properties on these presets to something
that breaks iD. This fix ensures that these "special" presets always
have the correct tags so that iD does not break.

* fix(MapExportDialog): fix React controlled/uncontrolled state warning (#459)

* fix: PDF reports (#456)

* chore(PDFReport): Code formatting

* feat(Reports): Fit images within a square

Allows easier calculation of pages prior to layout of images

* chore(Reports): Refactor PDF Report component

Avoid pass-through of props to components nested in components

* Allow navigation through entire report

* tweak CSS to show report paper outline clearly

* Smooth loading state transitions

* Cache and eagerly render PDF pages, and support initial page > 1

Support starting the report preview at a page > 1, which requires
previous pages to be rendered first in order to establish which
observation should appear on the current page. Also adds caching of
observation PDFs (each observation is rendered to its own PDF during
preview) and eagerly rendering the next observation, to reduce delay.

* chore: tidy up

* Add initialPageNumber option and add stories

* Add current page number

* chore: Small type fixes

* chore: Rename and add code comments

* fix: Queue PDF renders to use the same react-pdf instance

Previous code was causing React warnings of re-using the same context in
multiple renderers, because we created a new renderer instance for each
PDF preview doc (one for each observation). This changes the code to re-
use the same renderer, and update it with the doc for each page. It
should also be faster, since it's updating existing nodes rather than
re-creating the react tree.

* fix bug where loop could continue to infinity

* add missing dep to useEffect

* Update intl messages

* commit case change to git

* Add save button

* Add support for intl characters in PDF report

* fix webpack config to load fonts

* Add new design page navigator

* Format report toolbar and add x out of y observations count

* chore: fix types

* fix page navigator so that buttons are equally sized

* fix toolbar so that text is centered to page

* remove bottom border radius on page navigator

* 🎉 New report design & layout

* remove eager rendering (premature optimization, and caused problems)

* 🚀 page numbers in footer

* Fix re-render of PDF after cancelling edit of observation

* chore: Catch errors in PDF render and report them

* fix: Reset to page 1 when the user is on an invalid page

* chore: Change startPage to actually mean statePage (not zero-based idx)

* Turn eager rendering back on

* chore: Unused var

* Add storybook for observation with no location

* chore: Add types

* 🥳 Add icons to observations in report

* fix: patch hidden-mapbox so it does not hang

* fix: timeout PDF render

* fix: Cache category icon

* fix: Don't cache failed preview renders

* fix: pageIndex can end up with `null` values sometimes (not sure how!)

* fix: If preview is cancelled then bail

This can happen if the user changes the filter whilst the preview is
still loading. If the filter has changed, then we want to cancel async
functions that started with the previous filter

* fix: Catch infinite loop in preview

* feat: Use same map marker for observation as mapeo mobile

* fix: Fix patch to static map renderer

* fix: Only show fields with answers in report

* chore: Add Story for testing long preset names

* Add disabled state styling to page navigation buttons

* fix: Use new instance of ract-pdf for each render

Using the same renderer instance and updating the container was causing
race conditions, and does not seem to change performance much.

* chore: Use forked react-pdf/renderer

Changes babel preset-env to reduce polyfills and transpilation

* chore: Remove hidden-mapbox patch

* chore(release): 5.4.0-beta.11

* fix: Fix errors from Wifi Status display and disable on MacOS (#470)

Fixes #469

* fix: Fix "zoom to data": include deletions & correct density calc (#475)

* fix: Fix "zoom to data": include deletions and correct density calc

* fix: In territory view, zoom to data also considers observations

* chore: Update node version in Github Action

* feat: Update iD Editor background imagery (fix mapbox satellite) (#477)

Fixes #476

* fix: Fix crash when trying to resize sidebar (#478)

* fix: Fix crash when trying to resize sidebar

Fixes #473

* fix: Add button to un-collapse sidebar if collapsed

* fix: Fix Mapeo window randomly not opening at startup (#465)

* chore: Add incremental Typescript checking

Add // @ts-check to the top of files in src/background
and src/main to check JS files with JSDoc hints

* Refactor app startup code & background processes

* Manage background processes together, and time async startup actions

* Read backend state in main window

* Startup sequence in parallel

* remove console.log

* Fix startup sequence

* Fix background state updates (emit after state is updated)

* Update to latest electron@9

* Fix ipc message validation

* Fix settings extraction

* Wait for first render before showing main window

* Improve logging

* fixes for breaking mkdirp change

* add note about React devtools

* fix lint error

* chore(tsconfig): fix typo

* fix packaging config

* fix: Fix errors from Wifi Status display and disable on MacOS

Fixes #469

* fix: Package default config with app, rather than extracting at runtime

* fix: Fix IPC messaging with main process - call port.start()

* fix: Fix "zoom to data": include deletions and correct density calc

* fix: Fix crash when syncing by removing non-transferable objects from IPC

* Add message error handlers to IPC

* fix: Fix reload of Mapeo when config changes

* chore: update ecstatic & dedupe

* Fix package-lock

* Catch errors on startup

* Extend background process start timeout

* throw error if starting background process fails

* Pass through error events from background processes

* Cleanup subscriptions

* Clear closing timeout

Co-authored-by: Kira Oakley <kira@eight45.net>

* chore: Fix storybook

* fix: Don't allow observation location to be changed in territory view

Fixes #479

* chore: Remove unused files

* feat: Add Khmer, Dutch & French translations, fix th,pt,vi (#460)

* New translations en.json (Portuguese, Brazilian)

* New translations en.json (Portuguese, Brazilian)

* New translations en.json (Portuguese, Brazilian)

* New translations en.json (Portuguese, Brazilian)

* New translations en.json (Portuguese, Brazilian)

* New translations en.json (Portuguese, Brazilian)

* New translations en.json (Spanish)

* New translations en.json (Vietnamese)

* New translations en.json (Thai)

* New translations en.json (Dutch)

* New translations en.json (Dutch)

* New translations en.json (Sranan Tongo)

* New translations en.json (Sranan Tongo)

* New translations en.json (French)

* New translations en.json (French)

* New translations en.json (Khmer)

* New translations en.json (Khmer)

* New translations en.json (Khmer)

* New translations en.json (Khmer)

* New translations en.json (Khmer)

* New translations en.json (Khmer)

* New translations en.json (Khmer)

* New translations en.json (Khmer)

* New translations en.json (Thai)

* New translations en.json (Khmer)

* New translations en.json (Thai)

* New translations en.json (Thai)

* New translations en.json (Khmer)

* New translations en.json (Khmer)

* New translations en.json (Khmer)

* New translations en.json (Khmer)

* New translations en.json (Khmer)

* New translations en.json (Khmer)

* New translations en.json (Khmer)

* New translations en.json (Vietnamese)

* New translations en.json (Vietnamese)

* New translations en.json (Vietnamese)

* New translations en.json (Vietnamese)

* New translations en.json (Vietnamese)

* New translations en.json (Vietnamese)

* New translations en.json (Vietnamese)

* New translations en.json (Vietnamese)

* New translations en.json (Vietnamese)

* New translations en.json (Vietnamese)

* New translations en.json (Vietnamese)

* New translations en.json (Vietnamese)

* New translations en.json (Khmer)

* New translations en.json (Khmer)

* New translations en.json (Dutch)

* New translations en.json (Dutch)

* New translations en.json (Dutch)

Co-authored-by: Gregor MacLennan <gmaclennan@digital-democracy.org>

* fix: Fix npm ci failure in Github actions by retrying until it works

* chore: update to react-pdf latest (#484)

* chore: Update to @react-pdf/renderer@2.0.15

* Fix layout changes in react-pdf@2

* Fix: Move PDF reports to a web worker (#482)

* WIP: Move PDF reports to a web worker

* uri-templates

* Testing pdf in worker

* Fix inset maps and preset icons

* Fix: Show observation ID in mono-spaced font

* fix: Update import/export labels for territory data (#486)

* Update import data menu item label

* update territory export button label

* New translations en.json (Khmer)

* New translations en.json (Thai)

* New translations en.json (Thai)

* New translations en.json (Khmer)

* New translations en.json (Portuguese, Brazilian)

* New translations en.json (Portuguese, Brazilian)

* New translations en.json (Vietnamese)

* New translations en.json (Vietnamese)

* New translations en.json (Dutch)

* New translations en.json (Dutch)

* New translations en.json (Spanish)

* New translations en.json (Spanish)

* New translations en.json (French)

* New translations en.json (Spanish)

* New translations en.json (Dutch)

* New translations en.json (Vietnamese)

* New translations en.json (Portuguese, Brazilian)

* New translations en.json (Khmer)

* New translations en.json (Thai)

* New translations en.json (Portuguese, Brazilian)

Co-authored-by: Karissa McKelvey <633012+okdistribute@users.noreply.github.com>
Co-authored-by: okdistribute <karissa@noreply.github.com>
Co-authored-by: Gregor MacLennan <gmaclennan@digital-democracy.org>
Co-authored-by: Poga Po <poga.po@gmail.com>
Co-authored-by: Lou Huang <lou@louhuang.com>
Co-authored-by: Kira Oakley <kira@eight45.net>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

2 participants