-
Notifications
You must be signed in to change notification settings - Fork 1
Add template tenant #322
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 template tenant #322
Conversation
e589837 to
d5b9e88
Compare
|
Preview deployment: https://template-tenant.preview.avy-fx.org |
583df6a to
81e44b5
Compare
b2e8d26 to
010c057
Compare
busbyk
left a comment
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I like it!
We talked about some UI for duplicating all off the template tenant's (i.e. dvac) pages to a newly created tenant. I assume this is a precursor to that and that will be in another PR?
| // TODOs | ||
| // - Remove photos from blocks or use a global photo? |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Ah because photos are tenant scoped right. Hadn't thought about that.
I think using a global, fallback photo might be a nice UX. Might be worth calling this out in some helper text in the duplicate to drawer.
963bbb8 to
2e3276f
Compare
| layout, | ||
| title, | ||
| tenant, | ||
| slug: `${slug.replace(/^\/([^/]+)\//, `/${tenant.slug}/`)}-${timestamp}`, |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
When I duplicate a page to a tenant the slug is generated with the timestamp suffix. When viewing it in the pages list view I see that timestamped slug but in the edit view it shows as the slug determined from the fieldToUse which defaults to 'title' in https://github.com/NWACus/web/blob/template-tenant/src/fields/slug/SlugComponent.tsx

Then saving this results in a conflict:

I guess I have a few thoughts here:
- Whoa our slug field should probably allow us to modify the slug to be different than a slug determined from the configured
fieldToUsewhich is title for the pages collection. I think the behavior here is a bug with the slug field separate from this PR but does impact the functionality here. - Rather than generating a slug with a suffixed timestamp it would be nice to check if the slug is already taken and then append an incrementing number. I know that's more effort than just a guaranteed unique slug via the timestamp method. It would be nice to do this in a server action to be able to use the local API.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@busbyk I took the shortcut but agree adding a sequential/incremental number is so much better. I added a new endpoint to do all the heavy lifting for us in d554444.
I also side stepped the "slug bug" in payload where it is rendered based on the title since I added the sequential number to the title. This seemed like the cleanest approach.
d554444 to
04454ea
Compare
Co-authored-by: Kellen Busby <busbyk@users.noreply.github.com>
04454ea to
7e875d2
Compare
busbyk
left a comment
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I like the changes! Breaking this out into it's own endpoint will probably be helpful for future the script to copy all pages from the template tenant to a new tenant during onboarding.
Just one suggested change and a thought:
I've been colocating things (components, hooks, server actions, etc.) within the collection's folder if they are specific to a collection. What do you think about that? Do you like that approach?
It might be nice to colocate the DuplicatePageFor components and the duplicatePageToTenant handler inside of the pages collection for consistency if you think that makes sense.
i.e. ./src/collections/Pages/component/DuplicatePageFor and ./src/collections/Pages/endpoints/duplicatePageToTenant
Looks good pending the incrementing slug change (feel free to take a different approach than my suggestion of course but I do think we should handle copying the title and slug without an integer suffix if there is no conflict).
I like this approach better as well. If we do end up using it for posts or something else, we can unnest it from Pages. |
Co-authored-by: Kellen Busby <busbyk@users.noreply.github.com>
c3aa0b5 to
b552f70
Compare
Description
There is a request to help avalanche centers build there pages. The way we decided to do this is build a "template tenant" that will not be seen by anyone but super admins, create the pages under that tenant, and duplicate any pages to the respective avalance center that wants the page. This PR:
Duplicate page for...button in the edit view of pagesWe still need to alter permissions, but that will be a follow up PR with #316 out in the wild at the same time.
We also need to build the pages which will be a separate PR
Issue
Addressed part of of #243