Skip to content

Conversation

@thapacodes4u
Copy link
Contributor

Copy link
Member

@SlimDeluxe SlimDeluxe left a comment

Choose a reason for hiding this comment

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

Please see below comments before I move to the UI test.

Copy link
Member

@SlimDeluxe SlimDeluxe left a comment

Choose a reason for hiding this comment

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

  1. There's an error when creating a new section
Image
  1. Sections are not scoped to tenant? I can see the same sections on both sites.
  2. I don't know how but you missed the instruction to make sections as nav menu items. I even included a screenshot from the old app. But you checked as Done anyway.
Image
  1. Same goes for tenant test, marked as Done but I can't see it.

@SlimDeluxe
Copy link
Member

Before continuing please merge from the main branch, I updated the CI workflows.

@thapacodes4u
Copy link
Contributor Author

@SlimDeluxe Wait it already has all the changes from the main branch I don't know what you mean by. "Merge from the main branch"

@SlimDeluxe
Copy link
Member

@thapacodes4u your last commit in this PR was on 11th Aug, but I made a change to the CI files 2 days later. If you merge from main, you'll get the fixed workflows so that tests in the PR work.

@thapacodes4u
Copy link
Contributor Author

@SlimDeluxe This can be reviewed now

@SlimDeluxe
Copy link
Member

@thapacodes4u ... but you haven't made any changes? All the problems from the last comment are still present.

@thapacodes4u
Copy link
Contributor Author

@SlimDeluxe Ya you are right I thing I confused changes i did in the feat/menus with this one. Sorry for the issue. I will fix it.

@thapacodes4u
Copy link
Contributor Author

@SlimDeluxe Ready for review

@SlimDeluxe
Copy link
Member

There seems to be an error related to the tenant switcher somehow just by accessing the panel... When using the main branch, the panel works normally.

image

@thapacodes4u
Copy link
Contributor Author

There seems to be an error related to the tenant switcher somehow just by accessing the panel... When using the main branch, the panel works normally.

@SlimDeluxe I have fixed this issue but its in the core package (DataLinx/eclipsephp-core#27) here is the PR. You should pull this pr and try either merging that first or check with that.

Copy link
Member

@SlimDeluxe SlimDeluxe left a comment

Choose a reason for hiding this comment

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

  • Remove Pages link from nav
Image
  • Dynamic section list should be before the Sections link and use the icon that is currently set for Pages
Image
  • Pages management should be it's own page. It should not be done in the section. Some editors can have only right to create/update pages, but not sections, so these must clearly be separated. So please remove the page management from the section view and edit pages, and make it fully manageable on the dynamic page.
  • Improve the dynamic page:
    a) Use the section name for the page title and breadcrumb,
    b) Remove the Section column
Image
  • When I toggle some columns, the refreshed table shows pages from other sections too. E.g. adding the column code, table refreshes and shows all pages, even from other sections, but then when you reload the page, again the section filter is correctly applied

thapacodes4u and others added 3 commits September 30, 2025 11:39
-[x] Remove Pages link from navigation menu
-[x] Reorder dynamic section links before Sections link with document icon
-[x] Remove page management from section view (deleted PagesRelationManager)
-[x] Use section name for page title and breadcrumb on dynamic pages
-[x] Hide Section column when viewing pages filtered by section
-[x] Fix table filtering bug by moving section filter to getTableQuery
  - Add tenant-aware seeding for sections and pages
  - Create 5 sections with 2-5 random pages per tenant
  - Fix page code uniqueness using numeric pattern instead of words
  - Remove individual view permissions from PagePolicy and SectionPolicy
  - Remove view action from section edit page
  - Remove view from permission prefixes in resources
@thapacodes4u
Copy link
Contributor Author

@SlimDeluxe This is read for review.

Copy link
Member

@SlimDeluxe SlimDeluxe left a comment

Choose a reason for hiding this comment

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

Again some conflicts appeared, so please update your PR

@thapacodes4u
Copy link
Contributor Author

@SlimDeluxe This is read for review

Copy link
Member

@SlimDeluxe SlimDeluxe left a comment

Choose a reason for hiding this comment

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

There are some conflicts to resolve here.

@thapacodes4u
Copy link
Contributor Author

@SlimDeluxe Should be ready for review

Copy link
Member

@SlimDeluxe SlimDeluxe left a comment

Choose a reason for hiding this comment

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

  1. The seeder is generating too many sections per site (10). Let's do 2 per site instead.
  2. Short description (in page editing) should also be a WYS editor.
  3. When editing an existing page, you can't clear the "URL slug" field so that it'll be auto-generated from the new page title. E.g.:
  • edit page, change title to "abc"
  • the slug is not changed, which is correct
  • but also when you clear the slug and try to save, you can't, since the slug field is required, but also it does not auto-generate

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.

2 participants