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

Add new lines after error file URLs #1830

Merged

Conversation

t-wright
Copy link
Contributor

Description

Adds new lines after the error file URL and paths. Currently they are not clickable (in iTerm2 at least).

Pull request checklist

New release checklist

Steps to Test

  1. Check out PR.
  2. Run npm run build
  3. Run ./dist/bin/vip-import-media-status.js @<site_id>.production
  4. Respond n to the prompt Download file import errors report now?
  5. Command + click (or the equivalent in your terminal) to verify the link is clickable

@t-wright t-wright requested a review from a team May 10, 2024 06:19
Copy link
Contributor

github-actions bot commented May 10, 2024

Dependency Review

✅ No vulnerabilities or license issues or OpenSSF Scorecard issues found.

OpenSSF Scorecard

PackageVersionScoreDetails

Scanned Manifest Files

Copy link
Contributor

@saroshaga saroshaga left a comment

Choose a reason for hiding this comment

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

Thanks for fixing this!

image

I've wondering if we should remove the <link> from Downloading from <link> , to make it cleaner?

Also, @aagam-shah left some feedback yesterday that we should probably remove the & the data is retained for 7 days since the completion of the import phrase from the notice, and instead rely on the docs. What do you feel?

@t-wright
Copy link
Contributor Author

I've wondering if we should remove the <link> from Downloading from <link> , to make it cleaner?

I do think it's nice to be able to access the link. But perhaps it could be printed below the All errors have been exported to text if that might be neater? 🤔

@aagam-shah
Copy link
Contributor

Sorry if I was not clear, I meant that we should show the message that we have as-is even though the customer doesn't say YES to download the file because customer might not know the error file will not be available after 7 days if they don't download the file.

@saroshaga
Copy link
Contributor

But perhaps it could be printed below the All errors have been exported to text if that might be neater?

If we want to keep it, then it should be fine as is.

Copy link

Quality Gate Passed Quality Gate passed

Issues
0 New issues
0 Accepted issues

Measures
0 Security Hotspots
No data about Coverage
0.0% Duplication on New Code

See analysis details on SonarCloud

@saroshaga saroshaga merged commit 468ed26 into add/media-import-improvements May 10, 2024
15 checks passed
@saroshaga saroshaga deleted the update/media-import-error-url-printing branch May 10, 2024 11:03
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants