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

feat: Support Many to Many relationships #139

Open
wants to merge 17 commits into
base: master
Choose a base branch
from

Conversation

candidosales
Copy link

@candidosales candidosales commented Oct 10, 2023

What does it do?

  • getAllowedFields showing fields with relationship ManyToMany;
  • Improved regex in validatePattern to support fields with array. Ex: [categories[0].slug];
  • Generate sitemap correctly;

Why is it needed?

You can select multiple relationships and their attributes using this feature.

How to test it?

CleanShot.2023-10-09.at.23.11.44.mp4

Related issue(s)/PR(s)

#78

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

@candidosales candidosales changed the title [WIP] feat: Support to Many Relationshipg [WIP] feat: Support to Many Relationships Oct 10, 2023
@candidosales candidosales changed the title [WIP] feat: Support to Many Relationships [WIP] feat: Support Many to Many relationships Oct 10, 2023
server/services/pattern.js Outdated Show resolved Hide resolved
server/services/pattern.js Outdated Show resolved Hide resolved
@candidosales candidosales changed the title [WIP] feat: Support Many to Many relationships feat: Support Many to Many relationships Oct 10, 2023
@candidosales
Copy link
Author

@boazpoolman , could you review it? 😃

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.

Just basing this question off of your screenrecording;

Is it possible to select different indexes in the array? For example [relation[1].field]

@candidosales
Copy link
Author

@boazpoolman , it isn't possible. Because the validation pattern only accepts the exact allowed field names, we can improve it in the next PR.

CleanShot 2023-10-10 at 06 41 02@2x

@boazpoolman
Copy link
Member

I would suggest solving that in this PR.
It's an essential feature when we introduce the relation array in url patterns.

@candidosales
Copy link
Author

candidosales commented Oct 10, 2023

@boazpoolman , it's fixed in my latest commit 😃 . I'm fixing the lint issues

@candidosales
Copy link
Author

@boazpoolman , it's ready to review it

@boazpoolman
Copy link
Member

Great. I'll review as soon as I've got the time!

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.

The eslint job is still failing.

Also it's very hard to review this PR because you've auto-fixed the files based on some prettier config that does not originate from the plugin.

It would be amazing if you could revert all those changes so that the diff of this PR is simply the code needed to implement the Many to Many relation feature.

@candidosales
Copy link
Author

@boazpoolman It's ready to review.

The changes are:

  • getAllowedFieldsadded support to showing fields with relationship ManyToMany;
  • getFieldsFromPattern updated the Regex to support array fields;
  • resolvePatternadded support to interact with array values and your specific indexes;

Now, it's easier to review it.

@candidosales
Copy link
Author

@boazpoolman , could you review it?

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.

For starters:
Thanks @candidosales for submitting this PR.

I've reviewed it and I'm having some issues.

  1. If I use a toOne relation I see the dynamic value I can insert into the pattern (e.g. [category.id]). But if I add a toMany relation I don't see the field in the pattern description.
  2. If I add the dynamic value anyways (e.g. [category[0].id]) and I try to save I get an internal server error.

Please see my video in which you can witness these two issues:

sitemap.mp4

@candidosales
Copy link
Author

@boazpoolman , great! Thanks for reviewing it!

I'll take a look.

@boazpoolman
Copy link
Member

Hi @candidosales

Have you been able to look at my feedback?

@candidosales
Copy link
Author

Hi @boazpoolman , I didn't have time yet to improve it. Sorry about that. Many things are happening.

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.

None yet

2 participants