MemberJunction / MemberJunction/MJ

Adopt the UI Layering standard across all MemberJunction and Blue Cypress repos

Open
#3,404 0 comments 0 reactions 2 assignees Claimed by @MarceloT-BC View on GitHub
Dominant language
TSQL
Stars
29
Forks
6
Avg merge
2d 1h
Merged PRs (30d)
323

Description

@bc-izygmunt @MarceloT-BC — assigning this to the two of you to work through together.

## Context

MJ has built UI in layers three times and never written it down as a general standard, so every new app reinvents it — and the one place that *did* have a written rule drifted anyway. That is now fixed, with a guide **and** a gate:

| PR | What it does |
|---|---|
| **[MJ#3403](https://github.com/MemberJunction/MJ/pull/3403)** | `guides/UI_LAYERING_GUIDE.md` — the standard. Plus `.github/scripts/check-ui-layers.mjs`, the enforcement gate. |
| **[bizapps-accounting#37](https://github.com/MemberJunction/bizapps-accounting/pull/37)** | Worked example: the Journal Entry slice converted end to end. Found 3 real bugs. |
| **[bizapps-orders#23](https://github.com/MemberJunction/bizapps-orders/pull/23)** | Worked example: package split + the missing forms layer. Found 2 multi-provider bugs and 1 pre-existing red test. |

**Read the guide and at least one of the two app PRs before starting.** The app PRs are the pattern; the guide is the rule.

## The standard, in four lines

```
L3 Explorer surface entity forms + resource/dashboard components; owns NavigationService; NO domain logic
L2 Composite widget assembles L1; MAY read data, ONLY via ProviderToUse; NEVER navigates
L1 Presentational props in, events out; zero data access
L0 Domain runtime pure TS, no Angular at all
```

Two hard boundaries: nothing below L3 imports `@angular/router` or an Explorer package; nothing at L3 holds domain logic.

## Scope

Every MemberJunction repo, every Blue Cypress repo built on MJ (including **BCSaaS**), and any other org repo with an Angular UI on MJ.

- [ ] `MemberJunction/MJ` — 8 known-drifted `packages/Angular/Generic/**` packages, listed in `packages/Angular/Generic/CLAUDE.md`
- [ ] `MemberJunction/bizapps-common`
- [ ] `MemberJunction/bizapps-contracts`
- [ ] `MemberJunction/aidp`
- [ ] BCSaaS
- [ ] Remaining Blue Cypress org repos with an MJ Angular UI — please enumerate as a first step and add them here; this list is a starting point, not a survey

`bizapps-accounting` and `bizapps-orders` are covered by the PRs above, but each has a **checklist of remaining screens** in its own `docs/UI_LAYERING.md` §5/§6 — those belong in this effort too.

## Recommended approach

The two app PRs are deliberately shaped as a template, and the shape matters more than the speed:

1. **Turn the gate on first, opt-in.** Copy `.github/scripts/check-ui-layers.mjs` (single self-contained file, Node built-ins only, no MJ imports), add `check:ui-layers` to `package.json`, wire it into CI. Then declare `"mjUILayer"` on the packages that already comply. **Adding the field to a package that has drifted is how you find out what to fix** — that is the point of it being opt-in, not a way to avoid the work.
2. **Report the drift honestly rather than making the gate pass.** MJ#3403 declares 26 packages and names the 8 that fail, in `Generic/CLAUDE.md`. Silently skipping them would have made the gate a decoration.
3. **Find the duplicate first.** The strongest case for layering is two components rendering the same concept. In accounting it was the JE detail — two implementations, and merging them surfaced three bugs including a totals row one column out of alignment on a ledger screen. Extract that first; it pays for itself and makes the argument for the rest.
4. **One vertical slice at a time, fully, with tests.** A 40-file mechanical sweep teaches nothing and reviews badly. Each app PR converts exactly one slice and files the rest as a checklist.
5. **Write the repo's own `docs/UI_LAYERING.md`**, linking the MJ guide, with a prioritised checklist of what remains. Both app PRs have one to copy.

## Expected findings, based on the two PRs so far

Five bugs across two repos, none of which was visible from inside the file that contained it. Expect more of the same:

- a domain rule re-derived by hand next to a correct, unit-tested version of itself (and wrong in **both** directions);
- a `colspan` off by one, putting a debit total under the Credit heading;
- a drill-through reading a column that had been replaced, hidden by an untyped `ResultType: 'simple'` cast;
- `new RunView()` / `new Metadata()` in reusable components, silently ignoring the provider they were handed;
- a change-detection gap that freezes a view into empty states which read as "no data".

The last two classes are caught by the gate directly. The first three are what you find when you merge two implementations of the same thing.

## External guidance

The guide's §11 states this is also the **recommended pattern for external teams** building on MemberJunction, which is why the gate is one copyable file with no MJ dependencies. Worth a mention in the docs site / release notes once the internal sweep is under way.

Contributor guide

Open the contributing guide

Assessment

This issue has not been assessed yet.

Get new issues in your inbox

A short digest of beginner-friendly GitHub issues.