fix: Implemented the logic to force the user to tag at least one repositor… #2007

shamimbinnur · 2023-10-26T07:08:12Z

Description

This PR adds the logic to force the user to tag at least one repository if the system detects an empty array of repositories while creating a highlight.

The system has the logic to automatically detect repositories from the 'PR' and 'Issue' links and tag them to a highlight. However, for the dev.to blog, the system couldn't detect any related repositories, resulting in an empty array of repositories being passed to the backend. This is why an invalid repository was tagged from the backend.

To fix this issue, I have added logic to require users to manually tag at least one repository if the system detects an empty array of repositories while creating a highlight.

What type of PR is this? (check all applicable)

Related Tickets & Documents

Fixes #1995

Mobile & Desktop Screenshots/Recordings

Added tests?

👍 yes
🙅 no, because they aren't needed
🙋 no, because I need help

Added to documentation?

[optional] Are there any post-deployment tasks we need to perform?

[optional] What gif best describes this PR or how it makes you feel?

…y if the system detects an empty array of repositories while creating a highlight.

netlify · 2023-10-26T07:08:16Z

✅ Deploy Preview for oss-insights ready!

Name	Link
🔨 Latest commit	`ddcdc12`
🔍 Latest deploy log	https://app.netlify.com/sites/oss-insights/deploys/653a95f5b62b1500087f020c
😎 Deploy Preview	https://deploy-preview-2007--oss-insights.netlify.app
📱 Preview on mobile	Toggle QR Code... Use your smartphone camera to open QR code link.

To edit notification comments on pull requests, go to your Netlify site configuration.

netlify · 2023-10-26T07:08:17Z

✅ Deploy Preview for design-insights ready!

Name	Link
🔨 Latest commit	`ddcdc12`
🔍 Latest deploy log	https://app.netlify.com/sites/design-insights/deploys/653a95f50b67ee000727f848
😎 Deploy Preview	https://deploy-preview-2007--design-insights.netlify.app
📱 Preview on mobile	Toggle QR Code... Use your smartphone camera to open QR code link.

To edit notification comments on pull requests, go to your Netlify site configuration.

nickytonline

Thanks for the fix @shamimbinnur! We're just sorting out the wording and this should be good to merge.

nickytonline · 2023-10-26T12:45:21Z

components/molecules/HighlightInput/highlight-input-form.tsx

@@ -432,6 +432,12 @@ const HighlightInputForm = ({ refreshCallback }: HighlightInputFormProps): JSX.E

      const taggedRepoFullNames = taggedRepoList.map((repo) => `${repo.repoOwner}/${repo.repoName}`);

+      // Check if the user has tagged at least one repo and ask them to tag at least one if they haven't
+      if (taggedRepoList.length < 1) {
+        setError("Please tag at least one repo!");


wording (blocking): Make it clear why at least one repository needs to be added.

Suggested change

setError("Please tag at least one repo!");

setError("Please add at least one repository associated with your blog post");

@BekahHW, any suggestions for better wording?

Ok, I think this is tricky. I need some clarification first. The problem is that when someone submits a blog post, they are automatically generating a repo, right?

I think the repo should be optional in for blog post highlights. For example, it's still valid to highlight work on something like "A guide to writing documentation." There's not necessarily a repo associated with that. For the blog post mentioned in the issue, I'm not sure what repo I would tag.

Can we conditionally check if the highlight is a blog post, and then leave the input empty unless they put something in there?

@bdougie thoughts on the above?

If we move forward with this implementation, I think Nick's copy works just change the to your:

"Please add at least one repository associated with your blog post"

Good point that it needs to be when a blog post is added @BekahHW. Missed that. 😅

@shamimbinnur, we'd need to check that it's a blog post before showing that error.

Hey @nickytonline, thanks for suggesting the better wording.

Not allowing the user to post a blog without tagging the repo is intentional. As @bdougie's comment here states, "every blog highlight should be linked to a repo."

If no repo is tagged whether automatically or manually an empty array of repos is passed to the backend. However, the backend does not keep it empty; it just splits text from the blog link and creates an invalid repo URL.

Let me be clear, "Does every blog post highlight need to be linked to a repo?"

CC: @BekahHW @bdougie

Ok, so you're saying from a backend perspective, we have to have a repo listed?

Technically yes!

OgDev-01

Copy changes and we are good 🍕. Thanks for turning this around

## [1.73.0-beta.4](v1.73.0-beta.3...v1.73.0-beta.4) (2023-10-26) ### 🐛 Bug Fixes * add logic to force the user to tag at least one repository to a highlight ([#2007](#2007)) ([665fbf2](665fbf2))

…highlight (#2007)

## [1.73.0-beta.4](v1.73.0-beta.3...v1.73.0-beta.4) (2023-10-26) ### 🐛 Bug Fixes * add logic to force the user to tag at least one repository to a highlight ([#2007](#2007)) ([665fbf2](665fbf2))

## [1.73.0](v1.72.0...v1.73.0) (2023-10-26) ### 🍕 Features * implement improved UX for non-connected users profile ([#1992](#1992)) ([f11160c](f11160c)) ### 🧑‍💻 Code Refactoring * Improve UX on Create New List page - `/hub/lists/new` ([#1987](#1987)) ([c0fcf7a](c0fcf7a)) ### 🐛 Bug Fixes * add logic to force the user to tag at least one repository to a highlight ([#2007](#2007)) ([665fbf2](665fbf2)) * background color in text-input ([#1998](#1998)) ([1913462](1913462)) * more menu overlap issue on safari ([#1999](#1999)) ([66df010](66df010)) * now the error message in highlight form will alert assistive tech ([#2008](#2008)) ([a0d4b3b](a0d4b3b)) * set border radius for modals in small screens ([#2004](#2004)) ([738c714](738c714)) * show contributors name and company in table ([#2012](#2012)) ([79776a3](79776a3))

Implemented the logic to force the user to tag at least one repositor…

b26bae4

…y if the system detects an empty array of repositories while creating a highlight.

shamimbinnur changed the title ~~feat: Implemented the logic to force the user to tag at least one repositor…~~ bug: Implemented the logic to force the user to tag at least one repositor… Oct 26, 2023

Removed console log

cd9f150

shamimbinnur changed the title ~~bug: Implemented the logic to force the user to tag at least one repositor…~~ fix: Implemented the logic to force the user to tag at least one repositor… Oct 26, 2023

shamimbinnur marked this pull request as ready for review October 26, 2023 07:21

nickytonline reviewed Oct 26, 2023

View reviewed changes

nickytonline mentioned this pull request Oct 26, 2023

fix: now the error message in highlight form will alert assistive tech #2008

Merged

19 tasks

OgDev-01 approved these changes Oct 26, 2023

View reviewed changes

shamimbinnur added 3 commits October 26, 2023 22:32

Changed wording

bc10ca8

Wording changes

ed98a83

Wording changes

ddcdc12

brandonroberts approved these changes Oct 26, 2023

View reviewed changes

brandonroberts merged commit 665fbf2 into open-sauced:beta Oct 26, 2023
9 checks passed

nickytonline pushed a commit that referenced this pull request Oct 26, 2023

fix: add logic to force the user to tag at least one repository to a …

1da73b2

…highlight (#2007)

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

fix: Implemented the logic to force the user to tag at least one repositor… #2007

fix: Implemented the logic to force the user to tag at least one repositor… #2007

shamimbinnur commented Oct 26, 2023 •

edited

Loading

netlify bot commented Oct 26, 2023 •

edited

Loading

netlify bot commented Oct 26, 2023 •

edited

Loading

nickytonline left a comment

nickytonline Oct 26, 2023 •

edited

Loading

BekahHW Oct 26, 2023 •

edited

Loading

BekahHW Oct 26, 2023

BekahHW Oct 26, 2023

nickytonline Oct 26, 2023

shamimbinnur Oct 26, 2023

BekahHW Oct 26, 2023

shamimbinnur Oct 26, 2023

OgDev-01 left a comment

	setError("Please tag at least one repo!");
	setError("Please add at least one repository associated with your blog post");

fix: Implemented the logic to force the user to tag at least one repositor… #2007

fix: Implemented the logic to force the user to tag at least one repositor… #2007

Conversation

shamimbinnur commented Oct 26, 2023 • edited Loading

Description

What type of PR is this? (check all applicable)

Related Tickets & Documents

Mobile & Desktop Screenshots/Recordings

Added tests?

Added to documentation?

[optional] Are there any post-deployment tasks we need to perform?

[optional] What gif best describes this PR or how it makes you feel?

netlify bot commented Oct 26, 2023 • edited Loading

✅ Deploy Preview for oss-insights ready!

netlify bot commented Oct 26, 2023 • edited Loading

✅ Deploy Preview for design-insights ready!

nickytonline left a comment

Choose a reason for hiding this comment

nickytonline Oct 26, 2023 • edited Loading

Choose a reason for hiding this comment

BekahHW Oct 26, 2023 • edited Loading

Choose a reason for hiding this comment

BekahHW Oct 26, 2023

Choose a reason for hiding this comment

BekahHW Oct 26, 2023

Choose a reason for hiding this comment

nickytonline Oct 26, 2023

Choose a reason for hiding this comment

shamimbinnur Oct 26, 2023

Choose a reason for hiding this comment

BekahHW Oct 26, 2023

Choose a reason for hiding this comment

shamimbinnur Oct 26, 2023

Choose a reason for hiding this comment

OgDev-01 left a comment

Choose a reason for hiding this comment

shamimbinnur commented Oct 26, 2023 •

edited

Loading

netlify bot commented Oct 26, 2023 •

edited

Loading

netlify bot commented Oct 26, 2023 •

edited

Loading

nickytonline Oct 26, 2023 •

edited

Loading

BekahHW Oct 26, 2023 •

edited

Loading