diff --git a/CLAUDE.md b/CLAUDE.md index f4498c7..bbc341e 100644 --- a/CLAUDE.md +++ b/CLAUDE.md @@ -110,8 +110,7 @@ Flavors: iOS has `Dev`/`QA`/`Runner` schemes (`--flavor dev|qa` works on iOS). A 2. **Interfaces in `domain`, implementations in `data`.** Presentation talks to cubits and services, not to repositories or Dio. (`ui/custom/cookies.dart` violates this. It is a known exception, don't copy it.) 3. **Follow the established result pipeline.** Repositories return `ResultType`. Cubits extend - `BaseCubit` and use `onResult(...)` or a `switch` on `TSuccess`/`TError`. Do **not** chain - `mapSuccess`/`mapError` for side effects: `mapError` never calls its callback (known-issues #1). + `BaseCubit` and use `onResult(...)` or a `switch` on `TSuccess`/`TError`. 4. **Register everything in the owning package's `init.dart`.** Global cubits are singletons in `DomainInit`. Screen-scoped cubits should be created with `BlocProvider(create: ...)` at the page. 5. **Reuse before adding.** Check `common/core`, `BaseCubit`, `ListBlocState`, `PrimaryButton`, @@ -125,8 +124,8 @@ Flavors: iOS has `Dev`/`QA`/`Runner` schemes (`--flavor dev|qa` works on iOS). A 9. **Don't introduce a second pattern** (Riverpod, Provider-only state, another HTTP client, another DI container, freezed/json_serializable) without an explicit architecture decision. 10. **Don't edit generated or platform-generated files** (see below). -11. **Behavior changes need tests.** See [testing.md](docs/development/testing.md). There are no tests - yet, so create the package's `test/` directory as the guide describes rather than skipping. CI picks it up automatically. +11. **Behavior changes need tests.** See [testing.md](docs/development/testing.md). Only `common` and `domain` + have a few, so create the package's `test/` directory as the guide describes rather than skipping. CI picks it up automatically. 12. **Report pre-existing problems; don't silently fix them** in unrelated changes. Record them in known-issues.md instead. diff --git a/docs/architecture/known-issues.md b/docs/architecture/known-issues.md index 3df0e71..96a3891 100644 --- a/docs/architecture/known-issues.md +++ b/docs/architecture/known-issues.md @@ -7,36 +7,32 @@ because other docs reference them. ## Correctness -**#1 `ResultType.mapError` never calls its callback, so auth errors never reach the UI.** -`modules/common/lib/core/result_type.dart`: `mapError` returns `TError(e.error)` without invoking `error(...)`. -`AuthCubit.login` / `signUp` use `..mapSuccess(...)..mapError((f) => isError(f))`, so a `TError` leaves the -cubit in `RLoading` forever. It's latent today because the fake `AuthRepositoryImpl` always succeeds. It will -surface the moment a real backend is connected. Use `BaseCubit.onResult` or a `switch` until it's fixed. +**#1 Resolved (#118).** `ResultType.mapError` now invokes its callback, so `AuthCubit` emits `RError` on failure. **#2 The two environment mechanisms disagree.** -- `Environment.envConfigFile` loads `env/.` (`--dart-define ENV`, default `dev`). `EnvConfig.apiUrl` reads - `API_URL_`. The committed `env/.dev` defines `API_URL` (unsuffixed), so **the base URL is `''`**. +- `Environment.envConfigFile` loads `env/.` (`--dart-define ENV`, default `dev`), while `.env.example` documents a + single file with suffixed keys. `EnvConfig.apiUrl` accepts both (`API_URL_`, falling back to `API_URL`, since #119), + but the template still needs to settle on one layout. - `FlavorConfig.getEnvFilePath()` and `EnvConfig.envConfigFile` both return `env/.env.example` and are unused. - The doc comments tell you to create `env/.env` and add it to assets, but the code never loads `.env`. - `melos run run:web` runs the **prod** entrypoint (`lib/main.dart`) with `env/.dev`, so the flavor is PROD with dev values. - `app/pubspec.yaml` bundles all of `env/` as assets, so any secret placed there ships in the binary. - `Environment.clientSecret` / `portalUrl` read unsuffixed keys and are unused. -**#3 There are no tests, and CI fails before it checks anything.** -No package has a `test/` directory. `.github/workflows/sonar-qube-scann.yml` is enabled for PRs and pushes to `main`, +**#3 Almost no tests, and CI fails before it checks anything.** +The only tests are `modules/common/test` (`ResultType`) and `modules/domain/test` (`EnvConfig`). `app` and `data` have none. `.github/workflows/sonar-qube-scann.yml` is enabled for PRs and pushes to `main`, but its first step (`webfactory/ssh-agent`) fails because the `SSH_PRIVATE_KEY` secret isn't configured, so analyze, tests and SonarQube are skipped. No pub dependency is git-based, so the step isn't needed. The workflow also has no -format-check or build step. `sonar.tests` lists `app/test` and `modules/domain/test`, and neither exists. +format-check or build step. `sonar.tests` lists `app/test`, which doesn't exist. **#4 The analytics scaffolding isn't wired.** No `AnalyticsClient` is registered in GetIt, so any `TrackedPage` throws on first track. `routeObserver` isn't passed to `GoRouter(observers:)`, so the enter/exit events never fire. `FirebaseAnalytics` throws `UnimplementedError`, and `SetupAnalytics.initialize()` is never called. `firebase_core` is a dependency, but `Firebase.initializeApp` is commented out. -**#5 Build targets point to files that don't exist.** -`ios/Flutter/Release.xcconfig` has `FLUTTER_TARGET=lib/main/env/main.dart`, but the prod entrypoint is `lib/main.dart`. -`ios/qa.xcconfig` uses `FLUTTER_TARGET=lib/main/env/main_dev.dart` and `PREFIX=dev`. The README build commands use -`-t lib/main/env/main.dart --dart-define-from-file=env_prod.json`, which is a nonexistent path and file. +**#5 Resolved (#120).** The iOS Release and QA xcconfigs and the README build commands now point at existing entrypoints. +Still unverified with a real iOS build: the xcconfigs set `FLUTTER_TARGET` after including `Generated.xcconfig`, so they +may override the target passed with `flutter build -t`. **#6 `addModule.py` copies from the wrong path.** It clones `rootstrap/flutter-modules` into `/Users/Shared` (a hardcoded macOS path) but copies from @@ -74,7 +70,7 @@ configured, so it has nothing to do (and fails in packages without the dependenc **#13 Dependency hygiene.** - `intl_utils` (a code generator) is a runtime dependency of `app` and `common`, and `flutter_gen` is an unversioned `app` dev dependency that nothing uses. -- Test dependencies are declared but unused, because there are no tests (#3). +- Most test dependencies (`bloc_test`, `mocktail`, `build_runner`) are still unused (#3). **#14 Repository hygiene.** There are empty `.github/instructions/*.md.new` files. A stray `ios/Podfile` sits at the repo root: a default Flutter-generated diff --git a/docs/architecture/overview.md b/docs/architecture/overview.md index a809dad..613e1e5 100644 --- a/docs/architecture/overview.md +++ b/docs/architecture/overview.md @@ -78,8 +78,8 @@ Key types (all in `modules/common/lib/core/`): and `onResult(ResultType)`. `ListBlocState` extends it for list screens. `CancelableCubitMixin` (common) cancels in-flight futures on `close()`. -> Use `onResult` or a `switch` on the sealed `ResultType`. The `mapSuccess`/`mapError` chaining in -> `AuthCubit` looks correct but `mapError` never invokes its callback (known-issues #1). +> Prefer `onResult` or a `switch` on the sealed `ResultType`. `mapSuccess`/`mapError` also work for side effects +> (as in `AuthCubit`). `mapError` keeps the original error unless its callback returns an `Exception`. ## State ownership @@ -153,12 +153,11 @@ There are two mechanisms, and they don't fully agree. Understand both before cha |---|---|---| | Entrypoints | `app/lib/main.dart` (prod), `app/lib/main/env/main_dev.dart`, `main_qa.dart` | Construct `FlavorConfig(flavor: …)`, which sets `EnvConfig.env` to `DEV`/`QA`/`PROD` | | dotenv file choice | `Environment.envConfigFile` in `app/lib/main/env/env_config.dart` | Loads `env/.`, where `ENV` is the `--dart-define` `ENV` value (default `dev`) | -| API URL lookup | `EnvConfig.apiUrl` in `modules/domain/lib/env/env_config.dart` | Reads `dotenv.env['API_URL_']` | +| API URL lookup | `EnvConfig.apiUrl` in `modules/domain/lib/env/env_config.dart` | Reads `API_URL_`, falling back to `API_URL` | | Bundled files | `app/pubspec.yaml` assets: `env/` | Everything in `app/env/` ships inside the app bundle | -The committed `env/.dev` defines `API_URL` (no suffix) and `ENV=dev`, but `EnvConfig.apiUrl` looks for -`API_URL_DEV`, so **the Dio base URL is empty with the shipped files**. `env/.env.example` shows the -suffixed format. See known-issues #2 for the full list of drift (unused `getEnvFilePath`, the `.env` +The committed `env/.dev` defines `API_URL` (no suffix) and `ENV=dev`, which the fallback picks up. `env/.env.example` +shows the alternative single-file, suffixed format. See known-issues #2 for the full list of drift (unused `getEnvFilePath`, the `.env` naming in comments vs. `.dev` on disk, and the fact that bundled env files are readable by anyone with the binary). Platform flavor support: diff --git a/docs/development/bootstrap-customization.md b/docs/development/bootstrap-customization.md index 174213a..7ebe73c 100644 --- a/docs/development/bootstrap-customization.md +++ b/docs/development/bootstrap-customization.md @@ -35,11 +35,11 @@ This complements the step-by-step setup in the root `README.md`. ### Environments - [ ] Decide on one env scheme and fix the drift described in [overview.md § Environments](../architecture/overview.md#environments-and-flavors) - and known-issues #2. Today the committed `env/.dev` doesn't provide the `API_URL_DEV` key that `EnvConfig.apiUrl` reads. + and known-issues #2. `EnvConfig.apiUrl` accepts either `API_URL_` or `API_URL`. Pick one layout and document it. - [ ] Create env files per flavor, and don't commit real secrets. Everything in `app/env/` is bundled into the app as an asset. `SECRET_KEY` in `env/.dev` is a placeholder. -- [ ] Fix the flavor targets: `ios/qa.xcconfig` points `FLUTTER_TARGET` at `main_dev.dart` (with `PREFIX=dev`), and - `ios/Flutter/Release.xcconfig` points at the non-existent `lib/main/env/main.dart`. +- [ ] Check the iOS flavor targets (`FLUTTER_TARGET` in `ios/Flutter/*.xcconfig`, `ios/dev.xcconfig`, `ios/qa.xcconfig`) if you + add or rename entrypoints. - [ ] Android product flavors (if you need them). None exist today. - [ ] `NetworkConstants`: timeouts (2 s is aggressive), `tokenHeader` (`"token"`), and the example `productsPath` / `baseUrl`. @@ -73,8 +73,8 @@ This complements the step-by-step setup in the root `README.md`. `SSH_PRIVATE_KEY`, which is only needed for git-based pub dependencies (there are none), so either set it or drop the `ssh-agent` step. Without it, the job fails at that step (known-issues #3). - [ ] Bump the Flutter version with `fvm use ` and commit `.fvmrc`. CI follows it. -- [ ] `sonar-project.properties`: `projectKey`, `projectName`, `host.url`. `sonar.tests` lists `modules/domain/test`, - which doesn't exist yet. +- [ ] `sonar-project.properties`: `projectKey`, `projectName`, `host.url`. `sonar.tests` lists `app/test`, which doesn't + exist yet. - [ ] Add `melos run format` and a `flutter build` to CI. The current workflow runs neither (the README also mentions Bitrise and an RS-GPT-Review action, but neither is configured in this repo). - [ ] `.github/pull_request_template.md`: adjust the issue-tracker link. diff --git a/docs/development/feature-guide.md b/docs/development/feature-guide.md index 0156203..06cd89f 100644 --- a/docs/development/feature-guide.md +++ b/docs/development/feature-guide.md @@ -41,7 +41,6 @@ requirement - For lists with local add/remove, extend `ListBlocState`. - Mix in `CancelableCubitMixin` and wrap futures with `toCancelable(...)` when a request may outlive the screen. - For multi-variant state (like `AuthState`), use a sealed class and emit it as the `T` of `Resource`. - - Don't chain `mapSuccess`/`mapError` for side effects (known-issues #1). ## 2. Data (`modules/data/lib/`) diff --git a/docs/development/testing.md b/docs/development/testing.md index fce838d..bc172b5 100644 --- a/docs/development/testing.md +++ b/docs/development/testing.md @@ -2,7 +2,8 @@ ## Current state (verified) -- **There are no tests.** No package has a `test/` directory. +- **Very few tests:** `modules/common/test/core/result_type_test.dart` and `modules/domain/test/env/env_config_test.dart`. + `app` and `data` have no `test/` directory. - There are no integration tests (`integration_test/`), no golden tests, and no mocks or fakes checked in. - The test dependencies are declared but unused: - `app`: `flutter_test`, `bloc_test`, `mocktail`, `build_runner` @@ -46,7 +47,7 @@ These follow from the declared dev dependencies and the architecture. Keep new s - **Mocks**: use `mocktail` (no codegen). It's the only mocking library declared. Don't add `mockito`, which needs build_runner and produces generated `*.mocks.dart` files. - When you add a package's first tests, make sure `/test` is in `sonar.tests` in `sonar-project.properties`. It - currently lists `app/test` and `modules/domain/test`, which don't exist yet. + currently lists `app/test` (not created yet), `modules/domain/test` and `modules/common/test`. ## What a change must test