Skip to content

fix ui state not being consistent with the original request model #862

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.

Sign up for GitHub

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

Jump to bottom
Merged
merged 5 commits into from
Jul 11, 2025
Merged

fix ui state not being consistent with the original request model #862

merged 5 commits into from
Jul 11, 2025

Conversation

Udhay-Adithya
Copy link
Contributor

@Udhay-Adithya Udhay-Adithya commented Jul 1, 2025 •
edited
Loading

PR Description

  • The UI state remains consistent with the original request model
  • The user sees the request they originally created, not the modified version from scripts
  • The execution still uses the script-modified data for sending the actual HTTP request

Checklist

  • I have gone through the contributing guide
  • I have updated my branch and synced it with project main branch before making this PR
  • I am using the latest Flutter stable branch (run flutter upgrade and verify)
  • I have run the tests (flutter test) and all tests are passing

Added/updated tests?

  • Yes
  • No, and this is why: this is a minor fix

OS on which you have developed and tested the feature?

  • Windows
  • macOS
  • Linux

animator
animator reviewed Jul 1, 2025
View reviewed changes
lib/providers/collection_providers.dart Outdated
if (requestModel != null &&
!requestModel.preRequestScript.isNullOrEmpty()) {
requestModel = await handlePreRequestScript(
RequestModel executionRequestModel = requestModel!;
Copy link
Member

Choose a reason for hiding this comment

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

Why are you using assignment instead of creating a proper copy of request model which will be modified.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I've made the necessary corrections in the latest commit

animator
animator reviewed Jul 1, 2025
View reviewed changes
lib/providers/collection_providers.dart Outdated
RequestModel executionRequestModel = requestModel!;

if (!requestModel.preRequestScript.isNullOrEmpty()) {
executionRequestModel = await handlePreRequestScript(
requestModel,
Copy link
Member

Choose a reason for hiding this comment

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

Why are you passing the requestModel and the new executionRequestModel

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I've made the necessary corrections in the latest commit

animator
animator reviewed Jul 2, 2025
View reviewed changes
lib/screens/home_page/editor_pane/details_card/request_pane/request_scripts.dart Outdated
@@ -59,7 +59,7 @@ class _EditRequestScriptsState extends ConsumerState<EditRequestScripts> {
crossAxisAlignment: CrossAxisAlignment.center,
children: [
Padding(
padding: kPh8b6,
padding: EdgeInsets.symmetric(vertical: 5, horizontal: 8),
Copy link
Member

Choose a reason for hiding this comment

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

revert

@animator
Copy link
Member

animator commented Jul 2, 2025

@DenserMeerkat Please review.

@animator animator added the gsoc25 label Jul 2, 2025
DenserMeerkat
DenserMeerkat reviewed Jul 2, 2025
View reviewed changes
lib/providers/collection_providers.dart
@@ -281,9 +281,10 @@ class CollectionStateNotifier
return;
}

if (requestModel != null &&
Copy link
Contributor

@DenserMeerkat DenserMeerkat Jul 2, 2025 •
edited
Loading

Choose a reason for hiding this comment

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

Line 277

final EnvironmentModel? originalEnvironmentModel =
        ref.read(selectedEnvironmentModelProvider);

It should use activeEnvironmentIdStateProvider from settings_providers.dart to find the active EnvironmentModel and not selectedEnvironmentModelProvider from environment_providers.dart

using selectedEnvironmentModelProvider adds the variables to the wrong environment.

apidash.mp4

@Udhay-Adithya
Copy link
Contributor Author

Udhay-Adithya commented Jul 2, 2025 •
edited
Loading

When no particular environment is selected the following message will be logged in the console,

flutter: [JS WARN]: Setup Warning: injectedEnvironmentJson does not have the expected structure ({values: Array}). Using an empty environment. Received Data: {}
flutter: Skipped environment update as originalEnvironmentModel was null.
flutter: Warning: Post-response script updated environment variables, but no active environment was selected to save them to.

E022FC4A-FB5D-4617-967C-6D0B1FF26F45_1_201_a

@animator
Copy link
Member

animator commented Jul 2, 2025

When no particular environment is selected the following message will be logged in the console,

flutter: [JS WARN]: Setup Warning: injectedEnvironmentJson does not have the expected structure ({values: Array}). Using an empty environment. Received Data: {}
flutter: Skipped environment update as originalEnvironmentModel was null.
flutter: Warning: Post-response script updated environment variables, but no active environment was selected to save them to.

E022FC4A-FB5D-4617-967C-6D0B1FF26F45_1_201_a

Default environment is always Global.

@Udhay-Adithya
Copy link
Contributor Author

fix.mp4

@animator animator merged commit 991d486 into foss42:main Jul 11, 2025
@Udhay-Adithya Udhay-Adithya deleted the fix-scripts-state branch July 18, 2025 14:29
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@animator animator animator left review comments

+1 more reviewer

@DenserMeerkat DenserMeerkat DenserMeerkat left review comments

Reviewers whose approvals may not affect merge requirements
Assignees
No one assigned
Labels
gsoc25
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
3 participants
@Udhay-Adithya @animator @DenserMeerkat