Fixes #1378 Update Talawa to give a meaningful message if the URL isn't available. #1380

Ayush0Chaudhary · 2022-12-30T22:31:08Z

What kind of change does this PR introduce?

Issue Number:

Fixes #1378
and kind of fixes #1370

Did you add tests for your changes?
Yes

Snapshots/Videos:

Screenrecording_20221231_035010.mp4

If relevant, did you update the documentation?
Didn't update the docs.

Summary
In this PR if this

talawa/lib/widgets/organization_list.dart

Line 34 in 46cf4ef

if (result.hasException) {

If statement is true it gives a snackbar after 2sec It will prompt a snackbar. I have to add 2 seconds delay because we cannot prompt a snackbar while the widgets are building. Fix#1368

Does this PR introduce a breaking change?
NO

Other information
None

Have you read the contributing guide?
Yes

github-actions

Congratulations on making your first PR! 🎊 If you haven't already, check out our Contributing Guidelines and PR Reporting Guidelines to ensure that you are following our guidelines for contributing and creating PR.

…age if the URL isn't available.

palisadoes · 2022-12-31T01:04:15Z

Please fix the failing tests

lib/widgets/organization_list.dart

SiddheshKukade · 2022-12-31T03:37:16Z

@Ayush0Chaudhary actually, I can't understand the reason for adding 2-sec delay to the snackBar. We already have a Verify button to verify the server URL.

Ayush0Chaudhary · 2022-12-31T03:52:06Z

It only verify whether the url is working with http.get(uri) but https//:www.youtube.com will also clear this check

But in this implementation we check whether the data received is appropriate for us.

And 2 sec delay is because we cannot prompt snackbar if the widgets are still building

Ayush0Chaudhary · 2022-12-31T12:17:22Z

@SiddheshKukade I added these lines back
#1380 (comment)

Now the snackbar will be shown only once if the url added doesn't provide the correct data. Which otherwise would've just shown the loading icon

My goal here was to tell user that his url will not work.

And if timer wasn't added and showSnackBar was set to true
It shows this error on the screen

@Sagar2366

codecov-commenter · 2023-01-02T21:46:34Z

Codecov Report

Merging #1380 (21b2544) into develop (5b28a77) will increase coverage by 0.11%.
The diff coverage is 87.50%.

📣 This organization is not using Codecov’s GitHub App Integration. We recommend you install it so Codecov can continue to function properly for your repositories. Learn more

@@             Coverage Diff             @@
##           develop    #1380      +/-   ##
===========================================
+ Coverage    68.38%   68.50%   +0.11%     
===========================================
  Files          144      146       +2     
  Lines         7164     7207      +43     
===========================================
+ Hits          4899     4937      +38     
- Misses        2265     2270       +5

Impacted Files	Coverage Δ
lib/services/database_mutation_functions.dart	`6.12% <0.00%> (-0.13%)`	⬇️
lib/widgets/organization_list.dart	`45.28% <0.00%> (-2.72%)`	⬇️
lib/services/navigation_service.dart	`58.82% <100.00%> (+12.66%)`	⬆️
...model/pre_auth_view_models/set_url_view_model.dart	`100.00% <100.00%> (ø)`
lib/widgets/talawa_error_dialog.dart	`100.00% <100.00%> (ø)`
lib/widgets/talawa_error_widget.dart	`100.00% <100.00%> (ø)`
lib/views/pre_auth_screens/login.dart	`100.00% <0.00%> (ø)`
lib/views/pre_auth_screens/signup_details.dart	`0.00% <0.00%> (ø)`

📣 We’re building smart automated test selection to slash your CI/CD build times. Learn more

SiddheshKukade · 2023-01-04T10:24:39Z

@Ayush0Chaudhary Yes I got it thanks, changes are looking fine to me now.

Ayush0Chaudhary · 2023-01-04T10:49:37Z

@palisadoes @SiddheshKukade Please ask @Sagar2366 for 1 more review?
When I asked for re-review from @SiddheshKukade his review got removed?

SiddheshKukade · 2023-01-04T14:30:46Z

@palisadoes @SiddheshKukade Please ask @Sagar2366 for 1 more review? When I asked for re-review from @SiddheshKukade his review got removed?

@Ayush0Chaudhary I have already reviewed your changes.

Ayush0Chaudhary · 2023-01-04T14:34:17Z

but it is showing At least 2 approving reviews are required by reviewers with write access for merge

palisadoes · 2023-01-04T17:27:47Z

@Ayush0Chaudhary The code testing coverage has been reduced by -2.51% with this change. Please add the necessary tests to make sure that the coverage is maintained.

Ayush0Chaudhary · 2023-01-04T17:38:56Z

will do asap!

palisadoes · 2023-01-04T18:03:58Z

@Ayush0Chaudhary Is there a standardized function/class for all error messages? It would greatly help with creating a standardized look and feel for the app. Then we could create an issue to migrate all errors to the new methodology.

Is there a good best practice that we could follow for this?

SiddheshKukade · 2023-01-04T18:38:00Z

@Ayush0Chaudhary Is there a standardized function/class for all error messages? It would greatly help with creating a standardized look and feel for the app. Then we could create an issue to migrate all errors to the new methodology.

Is there a good best practice that we could follow for this?

@palisadoes Sir, we can create a separate custom widget for it and we can name that as TalawaErrorDialogue which will take a string to show the error message on the screen

palisadoes · 2023-01-04T18:43:15Z

@Ayush0Chaudhary Is there a standardized function/class for all error messages? It would greatly help with creating a standardized look and feel for the app. Then we could create an issue to migrate all errors to the new methodology.
Is there a good best practice that we could follow for this?

@palisadoes Sir, we can create a separate custom widget for it and we can name that as TalawaErrorDialogue which will take a string to show the error message on the screen

OK, @Ayush0Chaudhary please add this to this PR then we can open another issue to convert all our errors to use the new widget.

Ayush0Chaudhary · 2023-01-04T19:08:40Z

OK @palisadoes sir, I'll add this Widget

Ayush0Chaudhary · 2023-01-04T19:11:24Z

also we can specify custom error messages like below, just example


class Constants {
  /// USER AUTHENTICATION CONSTANTS
  static const String USER_AUTH_WRONG_CREDENTIALS =
      'Invalid Credentials, Please check.';
  static const String USER_AUTH_USER_NOT_FOUND =
      'User Not found, try Signing Up.';
  static const String USER_NOT_AUTHORIZED_TO_FETCH_USER =
      'You are not authorized to fetch this User';
  static const String USER_NOT_FOUND = 'No User Found';

  /// GENERIC FAILURE CONSTANTS
  static const String BAD_RESPONSE_FORMAT = 'Bad Response Format';
  static const String GENERIC_FAILURE =
      'Something Wrong Occured! Please try again.';
  static const String NO_INTERNET_CONNECTION = 'No Internet Connection';
  static const String HTTP_EXCEPTION = 'Unable to fetch response';
  static const String UNAUTHORIZED_EXCEPTION =
      'Unauthorized to fetch the resource.';
}

@palisadoes @SiddheshKukade

palisadoes · 2023-01-04T19:39:23Z

also we can specify custom error messages like below, just example


class Constants {
  /// USER AUTHENTICATION CONSTANTS
  static const String USER_AUTH_WRONG_CREDENTIALS =
      'Invalid Credentials, Please check.';
  static const String USER_AUTH_USER_NOT_FOUND =
      'User Not found, try Signing Up.';
  static const String USER_NOT_AUTHORIZED_TO_FETCH_USER =
      'You are not authorized to fetch this User';
  static const String USER_NOT_FOUND = 'No User Found';

  /// GENERIC FAILURE CONSTANTS
  static const String BAD_RESPONSE_FORMAT = 'Bad Response Format';
  static const String GENERIC_FAILURE =
      'Something Wrong Occured! Please try again.';
  static const String NO_INTERNET_CONNECTION = 'No Internet Connection';
  static const String HTTP_EXCEPTION = 'Unable to fetch response';
  static const String UNAUTHORIZED_EXCEPTION =
      'Unauthorized to fetch the resource.';
}

@palisadoes @SiddheshKukade

Remember that we need to ensure that the errors work in the selected language that user selects. The widget and supporting code / data will to be adjusted to support this.

Ayush0Chaudhary · 2023-01-05T20:34:49Z

@SiddheshKukade @palisadoes their already exist a function to show snack bar

talawa/lib/services/navigation_service.dart

Lines 52 to 63 in 46cf4ef

    
           void showSnackBar( 
        
             String message, { 
        
             Duration duration = const Duration(seconds: 2), 
        
           }) { 
        
             ScaffoldMessenger.of(navigatorKey.currentContext!).showSnackBar( 
        
               SnackBar( 
        
                 behavior: SnackBarBehavior.floating, 
        
                 duration: duration, 
        
                 content: Text(message), 
        
               ), 
        
             ); 
        
           }

Can you explain what you meant in
#1380 (comment)

import 'package:flutter/material.dart';
import 'package:get/get.dart';

class TalawaErrorWidget {
  static void showLight(String title, String message) {
    Get.snackbar(
      title,
      message,
      snackPosition: SnackPosition.BOTTOM,
      snackStyle: SnackStyle.FLOATING,
    );
  }

  static void showDark(String title, String message) {
    Get.snackbar(
      title,
      message,
      snackPosition: SnackPosition.BOTTOM,
      backgroundColor: Colors.black.withOpacity(0.8),
      colorText: Colors.white,
      snackStyle: SnackStyle.FLOATING,
    );
  }
}

Did you want something like this?

Ayush0Chaudhary · 2023-01-26T11:46:34Z

@noman2002 I did as you asked.

noman2002 · 2023-01-26T13:31:44Z

@Ayush0Chaudhary code coverage has gone down to 68.0 which is less than the minimum coverage required to merge a pr. Please try achieve 100% coverage for the file you have modified.

Ayush0Chaudhary · 2023-01-26T16:51:17Z

@noman2002 code coverage is 68.7 % now
I added the test for both the widget.

Ayush0Chaudhary · 2023-01-26T20:49:52Z

@palisadoes @noman2002 @TheHazeEffect @SiddheshKukade please review the PR

noman2002 · 2023-01-27T18:36:09Z

@Ayush0Chaudhary As the codecov is not getting triggered please attach a screenshot of code coverage of the files you have made changes, to get this PR approved. It will be easier for mentors to review.

Ayush0Chaudhary · 2023-01-27T21:04:15Z

For Set Url

Ayush0Chaudhary · 2023-01-27T21:07:36Z

For Talawa Error Widget (custom snackbar)

Ayush0Chaudhary · 2023-01-27T21:08:16Z

For talawa Error Dialog

Ayush0Chaudhary · 2023-01-27T21:10:05Z

For navigation service

the part I added

Ayush0Chaudhary · 2023-01-27T21:12:39Z

Overall coverage increased to 68.7% now

Ayush0Chaudhary · 2023-01-27T21:14:10Z

@noman2002 I added the code coverage of the test I added.
@palisadoes @TheHazeEffect @SiddheshKukade

noman2002 · 2023-01-27T21:20:35Z

@Ayush0Chaudhary Please remove the firebase api files it includes your api keys. Rest everything looks good to me.

Ayush0Chaudhary · 2023-01-27T21:28:03Z

@noman2002 done!! can we merge it now??

noman2002 · 2023-01-27T21:29:42Z

@noman2002 done!! can we merge it now??

Do we need to change anything in the installation process?

Ayush0Chaudhary · 2023-01-27T21:30:41Z

@noman2002 done!! can we merge it now??

Do we need to change anything in the installation process?

no, the process remains the same

noman2002

@palisadoes Looks good to me. Please have a look at it as there are various files changed.

feat: Add proper error handling if url not available.

9c563eb

github-actions bot reviewed Dec 30, 2022

View reviewed changes

Fix PalisadoesFoundation#1378 Update Talawa to give a meaningful mess…

fc5bd1a

…age if the URL isn't available.

Ayush0Chaudhary force-pushed the Ayush0Chaudhary/url-err-handling branch from c22d593 to fc5bd1a Compare December 30, 2022 22:38

palisadoes requested review from SiddheshKukade and Sagar2366 December 31, 2022 01:04

fixes the failed checks.

32863e8

SiddheshKukade requested changes Dec 31, 2022

View reviewed changes

lib/widgets/organization_list.dart Outdated Show resolved Hide resolved

Ayush0Chaudhary requested review from SiddheshKukade and removed request for Sagar2366 January 1, 2023 21:29

SiddheshKukade previously approved these changes Jan 4, 2023

View reviewed changes

Make the suggested changes

9a91238

Ayush0Chaudhary added 2 commits January 26, 2023 16:03

Merge branch 'develop' into Ayush0Chaudhary/url-err-handling

dd7552e

changed the app-setting test

e4b9b80

Add test for custom error widget and did err-handling in qrScan

01432d1

Ayush0Chaudhary force-pushed the Ayush0Chaudhary/url-err-handling branch from 72dee16 to 01432d1 Compare January 26, 2023 16:52

Ayush0Chaudhary mentioned this pull request Jan 26, 2023

Views: Create tests for signup_details.dart #1247

Closed

removed all warnings

732f22e

Ayush0Chaudhary added 5 commits January 28, 2023 02:53

Update build.gradle

be4e8a3

Delete google-services.json

9a897dc

Update build.gradle

f0a960e

Delete GoogleService-Info.plist

451f658

Delete firebase_app_id_file.json

21b2544

noman2002 approved these changes Jan 27, 2023

View reviewed changes

palisadoes approved these changes Jan 27, 2023

View reviewed changes

palisadoes requested a review from TheHazeEffect January 27, 2023 21:54

palisadoes merged commit eeabcb5 into PalisadoesFoundation:develop Jan 27, 2023

Fixes #1378 Update Talawa to give a meaningful message if the URL isn't available. #1380

Fixes #1378 Update Talawa to give a meaningful message if the URL isn't available. #1380

Conversation

Ayush0Chaudhary commented Dec 30, 2022 • edited Loading

github-actions bot left a comment

Choose a reason for hiding this comment

palisadoes commented Dec 31, 2022

SiddheshKukade commented Dec 31, 2022

Ayush0Chaudhary commented Dec 31, 2022 • edited Loading

Ayush0Chaudhary commented Dec 31, 2022 • edited Loading

codecov-commenter commented Jan 2, 2023 • edited Loading

Codecov Report

SiddheshKukade commented Jan 4, 2023

Ayush0Chaudhary commented Jan 4, 2023

SiddheshKukade commented Jan 4, 2023

Ayush0Chaudhary commented Jan 4, 2023

palisadoes commented Jan 4, 2023

Ayush0Chaudhary commented Jan 4, 2023

palisadoes commented Jan 4, 2023

SiddheshKukade commented Jan 4, 2023

palisadoes commented Jan 4, 2023

Ayush0Chaudhary commented Jan 4, 2023

Ayush0Chaudhary commented Jan 4, 2023

palisadoes commented Jan 4, 2023

Ayush0Chaudhary commented Jan 5, 2023 • edited Loading

Ayush0Chaudhary commented Jan 26, 2023

noman2002 commented Jan 26, 2023

Ayush0Chaudhary commented Jan 26, 2023

Ayush0Chaudhary commented Jan 26, 2023

noman2002 commented Jan 27, 2023

Ayush0Chaudhary commented Jan 27, 2023 • edited Loading

Ayush0Chaudhary commented Jan 27, 2023

Ayush0Chaudhary commented Jan 27, 2023

Ayush0Chaudhary commented Jan 27, 2023

Ayush0Chaudhary commented Jan 27, 2023

Ayush0Chaudhary commented Jan 27, 2023

noman2002 commented Jan 27, 2023

Ayush0Chaudhary commented Jan 27, 2023

noman2002 commented Jan 27, 2023

Ayush0Chaudhary commented Jan 27, 2023

noman2002 left a comment

Choose a reason for hiding this comment

Ayush0Chaudhary commented Dec 30, 2022 •

edited

Loading

Ayush0Chaudhary commented Dec 31, 2022 •

edited

Loading

Ayush0Chaudhary commented Dec 31, 2022 •

edited

Loading

codecov-commenter commented Jan 2, 2023 •

edited

Loading

Ayush0Chaudhary commented Jan 5, 2023 •

edited

Loading

Ayush0Chaudhary commented Jan 27, 2023 •

edited

Loading