Skip to content
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

[Android] Update submit validation API #4876

Merged
merged 22 commits into from
Oct 27, 2020
Merged

[Android] Update submit validation API #4876

merged 22 commits into from
Oct 27, 2020

Conversation

almedina-ms
Copy link
Contributor

@almedina-ms almedina-ms commented Oct 2, 2020 •
edited
Loading

Related Issue

Relates to #4829

Description

Updates the SubmitValidation API removing the use of the shared model InternalId, the usage of this properties led to app crashes due to the retrieval of the values taking too long.

The update also includes the registerSubmitableAction which lets the action listeners to know what actions should validation be performed for, this helps avoid unnecessary validations on custom elements that may not need it.

It also adds an areInputsValid which lets the developer know what the result of the last performed validation was, this removes the need to re-execute the validation process to know the validation result.

How Verified

The fix was verified by adding a sample CustomBlueAction and CustomActionListener which makes use of the previously described APIs

CustomBlueAction

Microsoft Reviewers: Open in CodeFlow

@shalinijoshi19
Copy link
Member

shalinijoshi19 commented Oct 7, 2020 •
edited
Loading

Related Issue

Please use one of the well-known github fixes keywords to reference
the issues fixed with this PR (eg Fixes #). If an issue doesn't yet exist please create one if reasonable to aid
in issue tracking.

NOTE: For multiple issues resolved by this PR use the corresponding keywords every time in a comma-delimited list per the reference
page above.

Description

Create a descriptive title and provide all of the relevant details in the description.
This information lives with the code and will help future readers (including yourself) and will serve as documentation.

At the very least please describe the issue you are addressing, and what your fix entails.

How Verified

How you verified the fix, including one or all of the following:

  1. New unit tests that were added if any. If none were added please add a quick line explaining why not.
  2. Existing relevant unit/regression tests that you ran
  3. Manual scenario verification if any; Do include .gif's or screenshots of the testing you performed here if you think that it
    will aid in code reviews or corresponding fixes on other platforms for eg.
Microsoft Reviewers: Open in CodeFlow

@almedina-ms can you remove the placeholder PR description text from the template and update with relevant description/details around this PR? thanks #Closed

shalinijoshi19
shalinijoshi19 suggested changes Oct 7, 2020
View reviewed changes
Copy link
Member

@shalinijoshi19 shalinijoshi19 left a comment

Choose a reason for hiding this comment

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

Could you start by updating the description? thanks

@ghost ghost removed the Needs: Author Feedback label Oct 9, 2020
@almedina-ms almedina-ms changed the title [Android] Update submit validation api [Android] Update submit validation API Oct 9, 2020
shalinijoshi19
shalinijoshi19 suggested changes Oct 12, 2020
View reviewed changes
Copy link
Member

@shalinijoshi19 shalinijoshi19 left a comment

Choose a reason for hiding this comment

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

Can you include a unit test for the public API to cover the crash scenarios to ensure automated coverage going forward? Thanks

@ghost ghost assigned RebeccaAnne Oct 17, 2020
@ghost
Copy link

ghost commented Oct 17, 2020

Hi @almedina-ms. This non-spec pull request has had no recent activity for the past 5 days . Please take the necessary actions (review, address feedback or commit if reviewed already) to move this along.

@ghost ghost removed the no-recent-activity label Oct 26, 2020
@ghost
Copy link

ghost commented Oct 26, 2020

Hi @almedina-ms; Thanks for taking action on your previously stale pull request. Resetting staleness.

@almedina-ms almedina-ms added this to the 20.10 milestone Oct 26, 2020
@ghost ghost removed the Needs: Author Feedback label Oct 26, 2020
@ghost ghost added the AdaptiveCards v20.10 label Oct 26, 2020
shalinijoshi19
shalinijoshi19 reviewed Oct 27, 2020
View reviewed changes
source/android/adaptivecards/src/androidTest/java/io/adaptivecards/objectmodel/APITest.java
import android.support.test.InstrumentationRegistry;
import android.support.v4.app.FragmentActivity;
import android.support.v4.app.FragmentManager;
Copy link
Member

Choose a reason for hiding this comment

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

if you are on here @almedina-ms can you add in the msft copyright header pls

shalinijoshi19
shalinijoshi19 approved these changes Oct 27, 2020
View reviewed changes
Copy link
Member

@shalinijoshi19 shalinijoshi19 left a comment

Choose a reason for hiding this comment

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

:shipit:

@almedina-ms almedina-ms merged commit b69b148 into main Oct 27, 2020
@almedina-ms almedina-ms deleted the user/almedina-ms/Android_UpdateSubmitValidationAPI branch October 27, 2020 23:57
@ghost
Copy link

ghost commented Oct 31, 2020

🎉AdaptiveCards@2020.10 has been released which fixes this issue.:tada:

Handy links:

rankush pushed a commit to rankush/AdaptiveCards that referenced this pull request May 8, 2024
@almedina-ms @shalinijoshi19 @golddove
[Android] Update submit validation API (microsoft#4876)
4e70ed0 
* Save Container Card Id in RenderArgs constructor

* Update Submit API

* Updates samples to mimic bug scenario

* Update documentation on the introduced APIs

* Fix casting bug

* Update API to be more similar to iOS

* Clear prevalidated inputs to avoid caching results

* Update the sample to add disabling

* Add first changes

* Add unit test and fix bug

* Add warning for test as focus makes it unusable

Co-authored-by: shalinijoshi19 <shalinij@microsoft.com>
Co-authored-by: Risheek Rajolu <risheekrr@gmail.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@shalinijoshi19 shalinijoshi19 shalinijoshi19 approved these changes

Assignees

@RebeccaAnne RebeccaAnne

Labels
None yet
Projects
None yet
Milestone
20.10
Development

Successfully merging this pull request may close these issues.
4 participants
@almedina-ms @shalinijoshi19 @RebeccaAnne @golddove