android-components icon indicating copy to clipboard operation
android-components copied to clipboard

For mozila-mobile/fenix#18565: Remove filtering of duplicate search requests

Open Yetoo1 opened this issue 4 years ago • 4 comments

Remove filtering of duplicate search requests

Pull Request checklist

  • [x] Quality: This PR builds and passes detekt/ktlint checks (A pre-push hook is recommended)
  • [ ] Tests: This PR includes thorough tests or an explanation of why it does not
  • [ ] Changelog: This PR includes a changelog entry or does not need one
  • [x] Accessibility: The code in this PR follows accessibility best practices or does not include any user facing features

After merge

  • Milestone: Make sure issues closed by this pull request are added to the milestone of the version currently in development.
  • Breaking Changes: If this is a breaking change, please push a draft PR on Reference Browser to address the breaking issues.

This doesn't have a test because the sole reason why this issue occurs is due to a filter catching changes between states localized in SearchFeature.kt. The simple removal of that filter with the effect localized within that file resolves that issue. If it's really needed, I can try to add it.

Yetoo1 avatar Apr 05 '21 14:04 Yetoo1

Codecov Report

Merging #10022 (2e26b85) into master (a69f74e) will decrease coverage by 0.55%. The diff coverage is n/a.

Impacted file tree graph

@@             Coverage Diff              @@
##             master   #10022      +/-   ##
============================================
- Coverage     74.13%   73.58%   -0.56%     
+ Complexity     5833     1047    -4786     
============================================
  Files           786      192     -594     
  Lines         29199     5538   -23661     
  Branches       4799      917    -3882     
============================================
- Hits          21646     4075   -17571     
+ Misses         5044     1031    -4013     
+ Partials       2509      432    -2077     
Impacted Files Coverage Δ Complexity Δ
...mozilla/components/feature/search/SearchFeature.kt 62.50% <ø> (-4.17%) 3.00 <0.00> (ø)
...components/lib/jexl/evaluator/EvaluatorHandlers.kt
...ava/mozilla/components/lib/crash/db/CrashEntity.kt
...components/feature/prompts/login/LoginSelectBar.kt
...ponents/feature/toolbar/ContainerToolbarFeature.kt
.../java/mozilla/components/lib/state/ext/Fragment.kt
...ents/feature/media/session/MediaSessionCallback.kt
...a/mozilla/components/service/fxa/AccountStorage.kt
...java/mozilla/components/support/base/facts/Fact.kt
...omponents/feature/prompts/provider/FileProvider.kt
... and 585 more

Continue to review full report at Codecov.

Legend - Click here to learn more Δ = absolute <relative> (impact), ø = not affected, ? = missing data Powered by Codecov. Last update a69f74e...2e26b85. Read the comment docs.

codecov[bot] avatar Apr 05 '21 14:04 codecov[bot]

Copying here the message from the Fenix issue (https://github.com/mozilla-mobile/fenix/issues/18565)

@Yetoo1 I understand this issue is important for you but let's wait until we hear from UX what should be the expected behavior. I'd say instead of making another search for the same search item that would give exactly the same results We should better not show the search option for the same exact search term. But again this is a decision for the UX team.

Mugurell avatar Apr 06 '21 11:04 Mugurell

This pull request has conflicts when rebasing. Could you fix it @Yetoo1? 🙏

mergify[bot] avatar Jun 28 '22 15:06 mergify[bot]

This pull request has conflicts when rebasing. Could you fix it @Yetoo1? 🙏

mergify[bot] avatar Sep 23 '22 20:09 mergify[bot]

This ended up being fixed in https://github.com/mozilla-mobile/android-components/pull/12363. Thanks again for your contribution and sorry for the overlap that had occurred there.

csadilek avatar Oct 17 '22 17:10 csadilek