Pp 749 gini demo app tl show attachments #784
Open
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
…yTableViewController` code PP-749
…ata to a file and update any existing data PP-749
…s in a local JSON file on disk PP-749
… info to show the transactionlist with attached documents PP-749
…alog and set it in the code PP-749
…d documents if exists PP-749
…and `documentPage for documentId` In DocumentService PP-749
…attached transaction documents and be accessed from outside of the SDK PP-749
…o be accessed from outside of the SDK PP-749
…in the TransactionList demo screen PP-749
…transaction from the list PP-749
…the document and set it if exists PP-749
… transaction date in the list of transactions PP-749
…oded name for other types of documents e.g `Document.png` PP-749
…iven documentId PP-749
… extractions in the invoice preview screen PP-749
… as in Figma design PP-749
…:gini/gini-mobile-ios into PP-749-Gini-Demo-app-TL-show-attachments
…to have `updateTransactionDocsViewModel` method to be used in multiple places - reduce duplicate code PP-749
…ap gesture internally PP-749
# Conflicts: # BankSDK/GiniBankSDK/Sources/GiniBankSDK/Core/GiniBankNetworkingScreenApiCoordinator.swift # BankSDK/GiniBankSDKExample/GiniBankSDKExample.xcodeproj/project.pbxproj
BankAPILibrary/GiniBankAPILibrary/Sources/GiniBankAPILibrary/Documents/DocumentService.swift
Show resolved
Hide resolved
...DK/GiniBankSDKExample/GiniBankSDKExample/TransactionDocs/TransactionListViewController.swift
Outdated
Show resolved
Hide resolved
...DK/GiniBankSDKExample/GiniBankSDKExample/TransactionDocs/TransactionListViewController.swift
Outdated
Show resolved
Hide resolved
BankSDK/GiniBankSDK/Sources/GiniBankSDK/Core/GiniBankConfiguration.swift
Outdated
Show resolved
Hide resolved
BankSDK/GiniBankSDK/Sources/GiniBankSDK/Core/GiniBankNetworkingScreenApiCoordinator.swift
Outdated
Show resolved
Hide resolved
.../Sources/GiniBankSDK/Core/TransactionDocs/Models/TransactionDocsDocumentPagesViewModel.swift
Outdated
Show resolved
Hide resolved
…(with: documentMetadata)` PP-749
…code duplication PP-749
…e duplication PP-749
…inate duplication PP-749
…duplication PP-749
…d DocumentServiceHelper - Added `DocumentServiceProviding` protocol to standardize document service interactions. - Introduced `DocumentServiceHelper` to handle document page fetching and processing. - Improved error handling and code reuse across the SDK. PP-749
…g and DocumentServiceHelper - Conformed `GiniBank` to `DocumentServiceProviding` for standardized document handling. - Replaced redundant document fetching logic with `DocumentServiceHelper`. - Simplified `loadDocumentData(for:)` and related methods to improve maintainability. - Ensured proper error handling and UI updates on the main thread. PP-749
…to use DocumentServiceHelper - Conformed `GiniBankNetworkingScreenApiCoordinator` to `DocumentServiceProviding`. - Removed duplicated document fetching methods in favor of `DocumentServiceHelper`. - Simplified `loadDocumentPagesAndHandleErrors(for:with:)` and other related methods. - Improved maintainability and ensured consistent document processing across the SDK. PP-749
|
zladzeyka
requested changes
Mar 7, 2025
There was a problem hiding this comment.
Thank you @ValentinaIancu-Gini, left you a couple of suggestions
|
|
||
| /// Initializes a `GiniTransactionDoc` instance. | ||
| /// | ||
| /// - Parameters: |
There was a problem hiding this comment.
[Suggestion]: I've noticed we used in our code base different comment styles,
majority uses "
/**
Initializes a new instance of GiniHealth.
This initializer creates a GiniHealth instance by first constructing a Client object with the provided client credentials (id, secret, domain)
- Parameters:
- id: The client ID provided by Gini when you register your application. This is a unique identifier for your application.
- secret: The client secret provided by Gini alongside the client ID. This is used to authenticate your application to the Gini API.
- domain: The domain associated with your client credentials. This is used to scope the client credentials to a specific domain.
- pinningConfig: Configuration for certificate pinning. Format ["PinnedDomains" : ["PublicKeyHashes"]]
- logLevel: The log level. `LogLevel.none` by default.
*/
"
There was a problem hiding this comment.
[Suggestion]: Please check the comment style we use in majority cases
"
/**
Initializes a new instance of GiniHealth.
This initializer creates a GiniHealth instance by first constructing a Client object with the provided client credentials (id, secret, domain)
- Parameters:
- id: The client ID provided by Gini when you register your application. This is a unique identifier for your application.
- secret: The client secret provided by Gini alongside the client ID. This is used to authenticate your application to the Gini API.
- domain: The domain associated with your client credentials. This is used to scope the client credentials to a specific domain.
- pinningConfig: Configuration for certificate pinning. Format ["PinnedDomains" : ["PublicKeyHashes"]]
- logLevel: The log level. `LogLevel.none` by default.
*/
"
| /// Retrieves the current view model for transaction documents. | ||
| /// - Returns: An optional `TransactionDocsViewModel` instance if available. | ||
| func getTransactionDocsViewModel() -> TransactionDocsViewModel? | ||
|
|
||
| /// Updates the `TransactionDocsViewModel` with new images and extracted data for a specific document. |
There was a problem hiding this comment.
[Suggestion]: Please check the comment style
| private var cameraInputImage: UIImage? { | ||
| return UIImage(named: "cameraInput")?.tintedImageWithColor(iconColor) | ||
| } | ||
|
|
| var image: UIImage { | ||
| return UIImage(named: rawValue).require(hint: "Image with name \(rawValue) missing") | ||
| } | ||
|
|
| containerView.trailingAnchor.constraint(equalTo: contentView.trailingAnchor, | ||
| constant: -Constants.containerHorizontalPadding), | ||
| containerView.topAnchor.constraint(equalTo: contentView.topAnchor, constant: 8), | ||
| containerView.bottomAnchor.constraint(equalTo: contentView.bottomAnchor, constant: -8), |
There was a problem hiding this comment.
[Suggestion]: Please wrap the magic numbers into constants
| paymentRecipientLabel.textColor = GiniColor(light: .GiniBank.dark1, dark: .GiniBank.light1).uiColor() | ||
| paymentRecipientLabel.font = UIFont.systemFont(ofSize: 17) | ||
|
|
||
| paymentPurposeLabel.font = UIFont.systemFont(ofSize: 13) |
There was a problem hiding this comment.
[Suggestion]: could you please wrap the numbers into Constants extension
| } | ||
|
|
||
| // Read the file | ||
| guard let data = try? Data(contentsOf: fileURL) else { return [] } |
There was a problem hiding this comment.
[Clarification]: what if we add FileManager.default.fileExists(atPath: fileURL.path) condition together in line 33, do we still need to create an empty file?
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
I've implemented the TransactionList screen to showcase the TDs feature.
Additionally, changes have been made to GiniAPILibrary to support requests using a documentId instead of the entire document object.
The TDs protocols have been updated by introducing new methods and properties required for the TransactionList. Some modifications were also made to the public protocol, including renaming and removing elements. However, these changes are not breaking, as this feature is not yet used in production.
For demo purposes, I chose to store transaction information in a JSON file, which led to the introduction of the FileManagerHelper class.
Changes were also made to the Demo screen of our example app:
A new Settings button was added (top right corner) to improve clarity and visibility.
A new button was introduced to display the TransactionList screen.
Colors were adjusted to align with the new design of this screen.