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

Multiple tree flow fix #210

Merged
merged 50 commits into from
Mar 24, 2021
Merged

Multiple tree flow fix #210

merged 50 commits into from
Mar 24, 2021

Conversation

ankitecd
Copy link
Contributor

Changes in this pull request:

  • Fixing multiple tree flow

@norbertschuler
Copy link
Collaborator

@ankitecd is this already ready to be tested?

@ankitecd
Copy link
Contributor Author

ankitecd commented Mar 9, 2021

@ankitecd is this already ready to be tested?

No, not yet

@norbertschuler

This comment has been minimized.

@norbertschuler
Copy link
Collaborator

norbertschuler commented Mar 11, 2021

  • The GeoJson export just exports the polygon, no sample trees or other information, right?

Simulator Screen Shot - iPhone 11 - 2021-03-11 at 13 24 25

Simulator Screen Shot - iPhone 11 - 2021-03-11 at 13 22 02Simulator Screen Shot - iPhone 11 - 2021-03-11 at 13 22 07

@norbertschuler

This comment has been minimized.

Comment on lines 108 to 109
camera &&
camera.current.setCamera({
Copy link
Collaborator

Choose a reason for hiding this comment

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

We should add check for all the other usages of the camera as well (I got a problem registering sample trees):

Bildschirmfoto 2021-03-12 um 10 37 13

@ankitecd
Copy link
Contributor Author

I tested multiple tree registrations with tags and got the following issues:

* [ ]  For the registered trees there is no possibility to enter a tag and so nothing is send with the API call to upload the registered trees and `tag: null` is returned - should it be possible to tag these trees?

* [ ]  For the sample trees if you enter the tag in the dialog at the same time when entering the diameter and height the tag gets send with the API call to upload the sample tree, but if you enter the tag later on the review page, it is not send anymore with the API call to upload a sample tree:

Simulator Screen Shot - iPhone 11 - 2021-03-22 at 10 35 10Simulator Screen Shot - iPhone 11 - 2021-03-22 at 10 35 47

  1. It is not possible to to tag multiple trees with one tag so tag for multiple tree is not required.
  2. Fixed in latest commit.

@ankitecd ankitecd linked an issue Mar 23, 2021 that may be closed by this pull request
@norbertschuler norbertschuler linked an issue Mar 23, 2021 that may be closed by this pull request
@norbertschuler
Copy link
Collaborator

norbertschuler commented Mar 23, 2021

  • I tested entering the data without being logged in in cm/m and then logged in with my account having countryCode=US and seeing the converted values - I miss some rounding with two places behind the dot (xxx.xx) for the inches/feet values:

Simulator Screen Shot - iPhone 11 - 2021-03-23 at 14 21 54Simulator Screen Shot - iPhone 11 - 2021-03-23 at 14 24 22Simulator Screen Shot - iPhone 11 - 2021-03-23 at 14 23 59

@norbertschuler
Copy link
Collaborator

norbertschuler commented Mar 23, 2021

  • The review page for a sample tree is not showing the photo if it is available.

Simulator Screen Shot - iPhone 11 - 2021-03-23 at 14 21 54Simulator Screen Shot - iPhone 11 - 2021-03-23 at 14 24 22Simulator Screen Shot - iPhone 11 - 2021-03-23 at 14 23 59

@norbertschuler
Copy link
Collaborator

norbertschuler commented Mar 23, 2021

  • The picture (confirmation for a location while specifying a polygon or just a hint for a picture) is only showing the lower part of the image with the woman - is that correct?

Simulator Screen Shot - iPhone 11 - 2021-03-23 at 18 16 11Simulator Screen Shot - iPhone 11 - 2021-03-23 at 18 20 42Simulator Screen Shot - iPhone 11 - 2021-03-23 at 18 29 57

@norbertschuler
Copy link
Collaborator

norbertschuler commented Mar 23, 2021

  • Species list has a green link for every number of species, but the link has not functionality (we should either remove the link or add the missing functionality):

Simulator Screen Shot - iPhone 11 - 2021-03-23 at 18 19 35

@norbertschuler
Copy link
Collaborator

norbertschuler commented Mar 23, 2021

  • I am missing the inch/feet conversion in the sample tree detail screen - or in the list - but at least these both differ:

Simulator Screen Shot - iPhone 11 - 2021-03-23 at 18 32 16!Simulator Screen Shot - iPhone 11 - 2021-03-23 at 18 32 21

@norbertschuler
Copy link
Collaborator

norbertschuler commented Mar 23, 2021

  • If I enter values 1 for height and diameter in feet and inches, on the review screen they are shown differently. So either the input or the output is wrong here too:

Simulator Screen Shot - iPhone 11 - 2021-03-23 at 18 38 08Simulator Screen Shot - iPhone 11 - 2021-03-23 at 18 38 13

@norbertschuler
Copy link
Collaborator

norbertschuler commented Mar 24, 2021

  • If I enter values 1 for height and diameter in feet and inches, on the review screen they are shown differently. So either the input or the output is wrong here too:

Simulator Screen Shot - iPhone 11 - 2021-03-23 at 18 38 08Simulator Screen Shot - iPhone 11 - 2021-03-23 at 18 38 13

  • Now 1 inch / 1 feet got converted into this on the review screen - even with some calculations a value of 1 should not be converted into a value of 0.98:

Simulator Screen Shot - iPhone 11 - 2021-03-24 at 13 46 00

@norbertschuler
Copy link
Collaborator

norbertschuler commented Mar 24, 2021

  • Species list has a green link for every number of species, but the link has not functionality (we should either remove the link or add the missing functionality):
  • There is a similar small UI issue on the species selection screen as the species is written in green color, but is not a link to do anything clicking on it.

Simulator Screen Shot - iPhone 11 - 2021-03-24 at 13 42 10

@norbertschuler
Copy link
Collaborator

norbertschuler commented Mar 24, 2021

  • If you do not change your location between point A and B, you get an alert, but it is shown on the next screen when you are ask to take picture and you can just click it away and ignore it - this is not really a bug, but also maybe not the wanted behavior @sagararyal:

Simulator Screen Shot - iPhone 11 - 2021-03-24 at 13 37 45Simulator Screen Shot - iPhone 11 - 2021-03-24 at 13 37 49

  • Not translating "Location" into "Standort" on the screen asking to take a photo is a bug :-)

@sonarcloud
Copy link

sonarcloud bot commented Mar 24, 2021

SonarCloud Quality Gate failed.

Bug A 0 Bugs
Vulnerability A 0 Vulnerabilities
Security Hotspot A 0 Security Hotspots
Code Smell A 44 Code Smells

No Coverage information No Coverage information
5.9% 5.9% Duplication

Copy link
Collaborator

@norbertschuler norbertschuler left a comment

Choose a reason for hiding this comment

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

All my issues seem to be solved, but I got 401 error while uploading the test data accumulated today and yesterday. Hitting the upload button twice seems to help, but I wonder how the OAuth token could get outdated between different uploads within a few seconds. Once I also got the error Bugsnag could not notify: error must be of type Error which might indicate that the parameter given to bugsnag.notify() somewhere in the code has the wrong type (also important to check for #262)?

I approved this PR now as I think these issues can be done in new PR not blocking this merge.

@sagararyal sagararyal merged commit d3a64f8 into develop Mar 24, 2021
@sagararyal sagararyal deleted the feature/multiple-trees-fix branch March 24, 2021 16:37
@norbertschuler norbertschuler mentioned this pull request Mar 24, 2021
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.

Register trees with inch and foot instead of cm and m Access to Mapbox items blocked
3 participants