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.

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

fix(tagging): fix downloading links with tags #2037

Merged
merged 1 commit into from
Oct 18, 2022

Conversation

halfwhole
Copy link
Collaborator

@halfwhole halfwhole commented Oct 18, 2022

Problem

To reproduce:

  1. Search by tags on the user page
  2. Click on "Download links"

Observed behaviour: downloaded links do not reflect the URLs searched by tags in the links table

Expected behaviour: downloaded links should reflect the URLs searched by tags in the links table

The bug happens because the function getUrls (used by the downloader) is unable to differentiate between searching by links vs searching by tags, and incorrectly always searches by links. This is unlike getUrlsForUser (used by the search bar), which has some extra added logic to differentiate between the two.

See Notion issue

Solution

Instead of mimicking the extra logic from getUrlsForUser in getUrls to differentiate between searching by links vs. by tags, this PR refactors the redux store such that this logic is no longer necessary. For context, searchText in the redux store is currently being overloaded for use in both searching by links and searching by tags. This PR refactors it by splitting it out into separate fields searchText and tags, which is better because it is consistent with the backend API.

I've also renamed searchTextInput to searchInput to disambiguate it from searchText. Now, searchInput can either be dispatched to searchText or tags, depending on the isTag boolean variable.

Tests

  • We don't have E2E tests for downloading links as CSVs. Might want to set that up sometime

Copy link
Contributor

@thanhdatle thanhdatle left a comment

Choose a reason for hiding this comment

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

It's a pretty elegant fix to me.

@halfwhole halfwhole merged commit d1201d9 into develop Oct 18, 2022
@halfwhole halfwhole deleted the fix/tagging/download-links-with-tags branch October 18, 2022 07:31
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