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

Integrate tldts to derive the domain name #7353

Merged
merged 9 commits into from
Jan 26, 2025
Merged

Conversation

brionmario
Copy link
Member

@brionmario brionmario commented Jan 21, 2025

Purpose

This pull request includes several changes to improve the handling of domain extraction and cookie management across multiple components and files. The most important changes involve replacing custom domain extraction functions with a more robust utility function from the tldts library.

Improvements to domain extraction:

Library and dependency updates:

These changes ensure more reliable and accurate domain extraction across the application, improving cookie management and overall functionality.

Related Issues

Related PRs

  • N/A

Checklist

  • e2e cypress tests locally verified. (for internal contributers)
  • Manual test round performed and verified.
  • UX/UI review done on the final implementation.
  • Documentation provided. (Add links if there are any)
  • Relevant backend changes deployed and verified
  • Unit tests provided. (Add links if there are any)
  • Integration tests provided. (Add links if there are any)

Security checks

pavinduLakshan
pavinduLakshan previously approved these changes Jan 21, 2025
Copy link

codecov bot commented Jan 21, 2025

Codecov Report

Attention: Patch coverage is 76.66667% with 7 lines in your changes missing coverage. Please review.

Project coverage is 41.98%. Comparing base (31b4e92) to head (688eb22).
Report is 469 commits behind head on master.

Files with missing lines Patch % Lines
modules/core/src/utils/url-utils.ts 53.33% 7 Missing ⚠️
Additional details and impacted files
@@            Coverage Diff             @@
##           master    #7353      +/-   ##
==========================================
+ Coverage   36.53%   41.98%   +5.45%     
==========================================
  Files          42       42              
  Lines         906      936      +30     
  Branches      223      214       -9     
==========================================
+ Hits          331      393      +62     
+ Misses        555      543      -12     
+ Partials       20        0      -20     
Flag Coverage Δ
@wso2is/core 41.98% <76.66%> (+5.45%) ⬆️

Flags with carried forward coverage won't be shown. Click here to find out more.

Files with missing lines Coverage Δ
modules/core/src/utils/storage-utils.ts 59.52% <100.00%> (+48.41%) ⬆️
modules/core/src/utils/url-utils.ts 85.48% <53.33%> (+57.82%) ⬆️

... and 5 files with indirect coverage changes

JayaShakthi97
JayaShakthi97 previously approved these changes Jan 22, 2025
@wso2-jenkins-bot
Copy link
Contributor

🦋 Changeset detected

The changes in this PR will be included in the next version bump.

Not sure what this means? Click here to learn what changesets are.

@brionmario brionmario merged commit 61590a6 into wso2:master Jan 26, 2025
11 checks passed
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.

Localization fails when domains from the Public Suffix List are included in the hostname
5 participants