Skip to content

Add save all transactions functionality#1214

Merged
cortinico merged 7 commits into
ChuckerTeam:mainfrom
irakliy01:add-save-transactions
May 8, 2024
Merged

Add save all transactions functionality#1214
cortinico merged 7 commits into
ChuckerTeam:mainfrom
irakliy01:add-save-transactions

Conversation

@irakliy01

@irakliy01 irakliy01 commented Apr 21, 2024

Copy link
Copy Markdown
Contributor

📷 Screenshots

image

📄 Context

I usually test my apps on an emulator that isn't connected to Google services. This means I can't directly share transactions with others, as I don't have suitable apps for that purpose. Instead, it's much easier for me to save transactions locally and then share the file from my computer using Device Explorer.
Chucker only supported sharing transaction details as text or HAR but lacked direct file saving capabilities (you could save only a one request/response body).
This PR adds a feature to save all transactions on the local storage. New save buttons are located within share menu separated from share buttons group by divider.

📝 Changes

  • Introduced "Save as Text" and "Save as HAR" buttons to the share menu, grouped together and visually separated from existing share options.
  • Implemented save-to-file logic, allowing users to save transactions data locally in either plain text or HAR format.
  • Developed a FileSaver object to streamline the file-saving process within the library.
  • Included a JUnit test to ensure the FileSaver.saveFile method writes files correctly to the given URI and with the expected content.

🛠️ How to test

Click on the share button in the app bar and then click on save as text / save as .har file. Check created files in a directory you specified.

* Impacted screen: Main transaction list
* New UI elements: Save buttons (grouped, separated from share buttons with the divider)
* Save options: Text file, HAR file
* Internal: Create FileSaver object for file writing
* Add kotlinx-coroutines-test dependency
* Test verifies correct file content is written using the provided URI
@irakliy01 irakliy01 requested a review from a team as a code owner April 21, 2024 23:04
Add a line to the unreleased block about save all transactions to the file

@cortinico cortinico left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

Thanks for sending this over @irakliy01
I believe we can include this 👍

assertThat(file.length()).isEqualTo(TEST_FILE_CONTENT.length)
assertThat(file.readText()).isEqualTo(TEST_FILE_CONTENT)

file.delete()

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

Why do you delete the file here? YOu should instead use a JUnit temp folder if you're afraid this will pollute other test runs

Comment on lines +16 to +17
): Boolean {
return withContext(Dispatchers.IO) {

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

Suggested change
): Boolean {
return withContext(Dispatchers.IO) {
): Boolean = withContext(Dispatchers.IO) {

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

Corrected

Comment on lines +430 to +431
): Source {
return when (type) {

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

Suggested change
): Source {
return when (type) {
): Source = when (type) {

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

Corrected

HAR -> saveHarToFile.launch(EXPORT_HAR_FILE_NAME)
}
},
onNegativeClick = null,

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

this is unneceesary?

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

There is no action on dismiss (old actions also set this listener as null)

@irakliy01

Copy link
Copy Markdown
Contributor Author

@cortinico updated PR

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.

2 participants