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

Feature/60 use same collection with different patterns #161

Merged

Conversation

TMSchipper
Copy link
Collaborator

What does it do?

Describe the technical changes you did.

Why is it needed?

Describe the issue you are solving.

How to test it?

Provide information about the environment and the path to verify the behaviour.

Related issue(s)/PR(s)

Let us know if this is related to any issue/pull request

@TMSchipper TMSchipper requested a review from boazpoolman August 15, 2024 18:41
Copy link

changeset-bot bot commented Aug 15, 2024

⚠️ No Changeset found

Latest commit: a187578

Merging this PR will not cause a version bump for any packages. If these changes should not result in a new version, you're good to go. If these changes should result in a version bump, you need to add a changeset.

This PR includes no changesets

When changesets are added to this PR, you'll see the packages that this PR includes changesets for and the associated semver types

Click here to learn what changesets are, and how to add one.

Click here if you're a maintainer who wants to add a changeset to this PR

@TMSchipper TMSchipper changed the title Feature/60 use same collection with different patterns Draft: Feature/60 use same collection with different patterns Aug 15, 2024
@TMSchipper TMSchipper marked this pull request as draft August 15, 2024 18:43
@TMSchipper TMSchipper force-pushed the feature/60-use-same-collection-with-different-patterns branch from 7ceb732 to 6b1e563 Compare August 27, 2024 21:04
@TMSchipper TMSchipper requested a review from boazpoolman August 27, 2024 21:04
@TMSchipper TMSchipper force-pushed the feature/60-use-same-collection-with-different-patterns branch from fa7f63b to 6623c50 Compare August 27, 2024 21:09
@TMSchipper TMSchipper force-pushed the feature/60-use-same-collection-with-different-patterns branch from 6623c50 to 3d9404c Compare August 27, 2024 21:10
@TMSchipper TMSchipper changed the title Draft: Feature/60 use same collection with different patterns Feature/60 use same collection with different patterns Aug 27, 2024
@TMSchipper TMSchipper self-assigned this Aug 28, 2024
@TMSchipper TMSchipper marked this pull request as ready for review August 28, 2024 09:06
Copy link
Member

@boazpoolman boazpoolman left a comment

Choose a reason for hiding this comment

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

Hi @TMSchipper
This looks really good, nice work.

I've left a couple comments regarding code style and some potentially unhandled bugs.

I also think you need to address the following things before we can get this merged:

  1. The host entity will still have a relation to only a single url_alias. We should probably update that to be a oneToMany relation instead of oneToOne. So then the entity will actually have a relation to all it's URL aliases.
  2. When we've established the oneToMany relation we need to have a way to define what is the primary URL alias of an entity. This is the URL alias which will be used when we're linking from one page to another. We need to have a single URL alias to make that link.
  3. We need to have a way for the Strapi Editor manage the multiple URL aliases. This needs to be developed in the admin panel. That way we can see the different URL aliases and manage them from within the edit form of a page.
  4. We probably need to update/add some unit tests for this new feature.

packages/core/server/admin-api/services/bulk-generate.ts Outdated Show resolved Hide resolved
packages/core/server/admin-api/services/bulk-generate.ts Outdated Show resolved Hide resolved
packages/core/server/admin-api/services/bulk-generate.ts Outdated Show resolved Hide resolved
packages/core/server/admin-api/services/bulk-generate.ts Outdated Show resolved Hide resolved
packages/core/server/admin-api/services/bulk-generate.ts Outdated Show resolved Hide resolved
packages/core/server/admin-api/services/url-pattern.ts Outdated Show resolved Hide resolved
packages/core/server/admin-api/services/url-pattern.ts Outdated Show resolved Hide resolved
packages/core/server/admin-api/services/url-pattern.ts Outdated Show resolved Hide resolved
packages/core/server/admin-api/services/url-pattern.ts Outdated Show resolved Hide resolved
packages/core/server/admin-api/services/url-pattern.ts Outdated Show resolved Hide resolved
Tim Schipper added 4 commits September 2, 2024 20:10
…ent-patterns

# Conflicts:
#	packages/core/server/admin-api/services/bulk-generate.ts
#	packages/core/server/admin-api/services/url-pattern.ts
@TMSchipper TMSchipper force-pushed the feature/60-use-same-collection-with-different-patterns branch from a444a18 to e8a273f Compare October 5, 2024 13:26
@TMSchipper TMSchipper force-pushed the feature/60-use-same-collection-with-different-patterns branch from 8c3f2c5 to c2e3559 Compare October 14, 2024 08:13
@TMSchipper TMSchipper force-pushed the feature/60-use-same-collection-with-different-patterns branch from c01fcb1 to e9f53c4 Compare October 20, 2024 11:20
@codecov-commenter
Copy link

codecov-commenter commented Oct 20, 2024

⚠️ Please install the 'codecov app svg image' to ensure uploads and comments are reliably processed by Codecov.

Codecov Report

All modified and coverable lines are covered by tests ✅

Project coverage is 37.96%. Comparing base (c32aee9) to head (a187578).

❗ Your organization needs to install the Codecov GitHub app to enable full functionality.

Additional details and impacted files
@@            Coverage Diff             @@
##           master     #161      +/-   ##
==========================================
- Coverage   38.14%   37.96%   -0.18%     
==========================================
  Files           4        4              
  Lines         839      864      +25     
  Branches      181      189       +8     
==========================================
+ Hits          320      328       +8     
- Misses        419      432      +13     
- Partials      100      104       +4     
Flag Coverage Δ
unit 37.96% <ø> (-0.18%) ⬇️

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

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

@boazpoolman boazpoolman merged commit d82e427 into master Nov 10, 2024
4 checks passed
@boazpoolman boazpoolman deleted the feature/60-use-same-collection-with-different-patterns branch November 10, 2024 17:01
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.

3 participants