Work on Changesets in API transmission #86

mariana0412 · 2024-11-18T17:11:41Z

According to the issue #84 :

Created ChangesetService:
- Provides methods for opening a changeset, uploading changes, and closing a changeset;
- Includes a changesetId property to track the currently opened changeset;
- Implemented as a Singleton to be accessible in both ContentView and AnnotationView.
Integrated ChangesetService in ContentView:
- A changeset is opened when ContentView appears.
Integrated ChangesetService in AnnotationView:
- Changes are uploaded to the currently opened changeset when the user taps the "Next" button;
- The changeset is closed after the user processes the last frame.
Updated the button label in AnnotationView:
- The "Next" button changes to "Finish" when the user reaches the last frame.

… on AnnotationView screen

himanshunaidu · 2024-11-21T00:11:39Z

IOSAccessAssessment/Services/ChangesetService.swift

+
+class ChangesetService {
+
+    private enum Constants {


Just a small point, but we can later move the constants to the global constants file, for better centralization, or at least in a common folder for constants.

himanshunaidu · 2024-11-21T00:12:58Z

IOSAccessAssessment/Services/ChangesetService.swift

+        static let workspaceId = "168"
+    }
+
+    static let shared = ChangesetService()


I like the idea of using singletons, and imo we should have used something like that for the SharedImageData as well (along with adding it to a global store, instead of passing it to every view)

himanshunaidu · 2024-11-21T00:23:08Z

IOSAccessAssessment/Services/ChangesetService.swift

+        """
+        <osmChange version="0.6" generator="GIG Change generator">
+            <create>
+                <node id="-1" lat="\(nodeData.latitude)" lon="\(nodeData.longitude)" changeset="\(changesetId)" />


We will have to of course, work on generalizing this on any kind of output that we get from the post-processed output of AnnotationView.

himanshunaidu · 2024-11-21T00:28:06Z

IOSAccessAssessment/Views/ContentView.swift

+        }
+        .navigationBarTitle("Camera View", displayMode: .inline)
+        .onAppear {
+            openChangeset()


Just to confirm, this is the only change that has been added to openChangeset right?
Kinda confused due to the spacing changes.

If the change set creation fails, then I am not sure we should be proceeding with the ContentView recording since it will be for nothing.
Let's see what kind of response we can decide upon if that happens.
I am not a fan of crashing the application, but we will have to have a retry option.

himanshunaidu · 2024-11-21T00:29:12Z

The changes look good to me as they are.
I am not currently sure if we should be merging this immediately, maybe we will.
Let's give it some time before making the call.
I am hoping to merge my own class correction PRs, and we will check accordingly.

himanshunaidu · 2024-11-21T19:20:22Z

So I had to push my own PR which was a long time due.
This has caused some conflicts in ContentView.
Be wary while resolving the merge conflicts. I have changed the other views such as AnnotationView quite significantly, so the code in ContentView that reference these views are also changed.

himanshunaidu · 2024-12-11T08:12:09Z

IOSAccessAssessment/Services/ChangesetService.swift

+        guard let xmlData = """
+            <osm>
+                <changeset>
+                    <tag k="created_by" v="GIG 1.0(4)" />


In the new PR that we raise for change sets (I think we will have to considering the conflicts), we should change this to match our own arguments.
Maybe for now: created_by = iOSPointMapper
comment can be left blank for now.

Both "created_by" and "comment" are according to the documentation (4 b Open a changeset): https://docs.google.com/document/d/1qPSW5pYmVb-RXOiryBm452sLfTvFl38f_TCO4SElkSY/edit?tab=t.0

I resolved merge conflicts

Yes, but that example input is for GoInfoGame. We are a different creator, so we should be having our own identifier.

Ah cool. Let's proceed with this PR then

himanshunaidu · 2024-12-12T06:03:46Z

IOSAccessAssessment/Services/ChangesetService.swift

+    private(set) var changesetId: String?
+
+    func openChangeset(completion: @escaping (Result<String, Error>) -> Void) {
+        guard let url = URL(string: "\(Constants.baseUrl)/changeset/create"),


So the fact that we now have two Constants structs will lead to confusion.
No problem tho, we will move all this to an ideal place in a future refactor.

This is not a problem because the Constants enum is private for each struct

himanshunaidu · 2024-12-12T06:56:43Z

IOSAccessAssessment/Services/ChangesetService.swift

+        }.resume()
+    }
+
+    func closeChangeset(completion: @escaping (Result<Void, Error>) -> Void) {


I think if we should be closing the ChangeSet in the ContentView as well, every time the back button is clicked.

Nevermind. Change sets are generally closed automatically when this happens

himanshunaidu

I am still approving and merging this request for now as it does what it is intended to do, but in the second PR we will take the comments into account.

mariana0412 added 8 commits November 18, 2024 18:23

Implement opening a changeset

3585de0

Open a changeset when the ContentView appears

e7ca4a6

Remove unnecessary trimming changesetId

61dac3f

Implement uploading changes

71aebed

Upload changes with latitude and longitude after each class assigning…

9d00b66

… on AnnotationView screen

Create NodeData struct for keeping together the parameters for pushing

4da7da3

Implement closing a changeset

3822958

Close a changeset once the user validates all the frames

f04a339

mariana0412 requested a review from himanshunaidu November 18, 2024 17:11

mariana0412 self-assigned this Nov 18, 2024

himanshunaidu changed the base branch from main to feature/login November 20, 2024 23:56

himanshunaidu changed the base branch from feature/login to main November 20, 2024 23:56

himanshunaidu reviewed Nov 21, 2024

View reviewed changes

himanshunaidu reviewed Dec 11, 2024

View reviewed changes

mariana0412 added 2 commits December 11, 2024 18:36

Merge branch 'main' into feature/manage-changesets

5dda9ea

Remove redundant printing

81d8ad2

himanshunaidu reviewed Dec 12, 2024

View reviewed changes

himanshunaidu approved these changes Dec 12, 2024

View reviewed changes

himanshunaidu merged commit 51bd3f9 into main Dec 12, 2024

himanshunaidu linked an issue Dec 12, 2024 that may be closed by this pull request

Work on Changesets in API transmission #84

Closed

himanshunaidu deleted the feature/manage-changesets branch January 8, 2025 22:18

Work on Changesets in API transmission #86

Work on Changesets in API transmission #86

Uh oh!

Conversation

mariana0412 commented Nov 18, 2024

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

himanshunaidu commented Nov 21, 2024

Uh oh!

himanshunaidu commented Nov 21, 2024

Uh oh!

Choose a reason for hiding this comment

Uh oh!

mariana0412 Dec 11, 2024 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

himanshunaidu Dec 12, 2024 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

himanshunaidu left a comment

Choose a reason for hiding this comment

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

3 participants

mariana0412 Dec 11, 2024 •

edited

Loading

himanshunaidu Dec 12, 2024 •

edited

Loading