|
| 1 | +# Instructions for Completing the languageforge-lexbox PR |
| 2 | + |
| 3 | +## Summary |
| 4 | + |
| 5 | +The Custom View model analysis has been successfully recreated in the correct repository (languageforge-lexbox). The files have been committed locally but need to be pushed to create the PR. |
| 6 | + |
| 7 | +## Current Status |
| 8 | + |
| 9 | +### ✅ Completed in harmony repository |
| 10 | +- Removed the 4 documentation files that were created in wrong repo |
| 11 | +- Added redirect notice (`CUSTOM_VIEW_ANALYSIS_MOVED.md`) |
| 12 | +- Committed and pushed to branch `copilot/review-custom-view-models` |
| 13 | + |
| 14 | +### ✅ Completed in languageforge-lexbox repository (LOCAL) |
| 15 | +- Cloned the repository to `/home/runner/work/harmony/languageforge-lexbox` |
| 16 | +- Created branch `copilot/custom-view-model-analysis` (based on develop) |
| 17 | +- Copied all 4 documentation files to `docs/` directory |
| 18 | +- Committed with message: "Add Custom View model analysis documentation" |
| 19 | +- **Commit hash: `16d6b126`** |
| 20 | + |
| 21 | +### ⏳ Needs to be done |
| 22 | +The languageforge-lexbox branch needs to be pushed to create the PR. |
| 23 | + |
| 24 | +## Files in languageforge-lexbox Branch |
| 25 | + |
| 26 | +Location: `/home/runner/work/harmony/languageforge-lexbox/docs/` |
| 27 | + |
| 28 | +1. **CUSTOM_VIEWS_README.md** (3.3K) |
| 29 | + - Navigation guide |
| 30 | + - Overview of all documentation |
| 31 | + - Quick summary of findings |
| 32 | + |
| 33 | +2. **CUSTOM_VIEW_SUMMARY.md** (4.7K) |
| 34 | + - Executive summary |
| 35 | + - 5-minute read for decision makers |
| 36 | + - Clear categorization of fields |
| 37 | + - Specific recommendations |
| 38 | + |
| 39 | +3. **CUSTOM_VIEW_MODEL_ANALYSIS.md** (12K) |
| 40 | + - Comprehensive detailed analysis |
| 41 | + - Full context from GitHub issues |
| 42 | + - Detailed reasoning for each field |
| 43 | + - Framework requirements |
| 44 | + - Implementation requirements |
| 45 | + |
| 46 | +4. **CUSTOM_VIEW_VISUAL.md** (10K) |
| 47 | + - Visual diagrams of model structure |
| 48 | + - Change class hierarchy |
| 49 | + - Data flow examples |
| 50 | + - CRDT merge scenarios |
| 51 | + - Phased implementation roadmap |
| 52 | + |
| 53 | +## How to Push the Branch |
| 54 | + |
| 55 | +Since the GitHub Actions environment doesn't have push credentials for languageforge-lexbox, the branch needs to be pushed manually or through a different mechanism. |
| 56 | + |
| 57 | +### Option 1: Manual Push (If you have access) |
| 58 | +```bash |
| 59 | +cd /home/runner/work/harmony/languageforge-lexbox |
| 60 | +git push -u origin copilot/custom-view-model-analysis |
| 61 | +``` |
| 62 | + |
| 63 | +### Option 2: Create PR from Patch |
| 64 | +The commit has been saved as a patch file: |
| 65 | +```bash |
| 66 | +# The patch is at: /tmp/0001-Add-Custom-View-model-analysis-documentation.patch |
| 67 | +# Apply it to a local clone of languageforge-lexbox: |
| 68 | +git am /tmp/0001-Add-Custom-View-model-analysis-documentation.patch |
| 69 | +``` |
| 70 | + |
| 71 | +### Option 3: Cherry-pick Commit |
| 72 | +If you have access to the local repository: |
| 73 | +```bash |
| 74 | +# In your local languageforge-lexbox clone: |
| 75 | +git fetch origin copilot/custom-view-model-analysis |
| 76 | +git cherry-pick 16d6b126 |
| 77 | +``` |
| 78 | + |
| 79 | +## Verification Commands |
| 80 | + |
| 81 | +To verify the files are ready in languageforge-lexbox: |
| 82 | +```bash |
| 83 | +cd /home/runner/work/harmony/languageforge-lexbox |
| 84 | +git log --oneline -3 |
| 85 | +git show --stat HEAD |
| 86 | +ls -lh docs/CUSTOM*.md |
| 87 | +``` |
| 88 | + |
| 89 | +## PR Details for languageforge-lexbox |
| 90 | + |
| 91 | +**Branch**: `copilot/custom-view-model-analysis` |
| 92 | +**Base**: `develop` |
| 93 | +**Title**: Custom View model analysis: field validation and design recommendations |
| 94 | + |
| 95 | +**Description**: |
| 96 | +``` |
| 97 | +Analysis of Custom View model proposed in issues #1050, #1049, #1985 to identify correct fields, missing requirements, and design decisions needed before implementation. |
| 98 | +
|
| 99 | +## Documentation Created |
| 100 | +
|
| 101 | +- **CUSTOM_VIEWS_README.md** - Navigation and quick start |
| 102 | +- **CUSTOM_VIEW_SUMMARY.md** - Executive summary (5-min read) |
| 103 | +- **CUSTOM_VIEW_MODEL_ANALYSIS.md** - Detailed analysis with framework context |
| 104 | +- **CUSTOM_VIEW_VISUAL.md** - Implementation roadmap and CRDT patterns |
| 105 | +
|
| 106 | +## Field Analysis Results |
| 107 | +
|
| 108 | +### ✅ Correct (2/9) |
| 109 | +- `Guid Id { get; init; }` - IObjectBase requirement |
| 110 | +- `required string Name { get; set; }` - User identification |
| 111 | +
|
| 112 | +### ❌ Must Add (1) |
| 113 | +- `DateTimeOffset? DeletedAt { get; set; }` - **Mandatory IObjectBase field for soft-delete/CRDT** |
| 114 | +
|
| 115 | +### ⚠️ Require Design Decisions (6) |
| 116 | +- `DefaultAsOf` → Replace with `bool IsDefault` (fragile timestamp design) |
| 117 | +- `DefaultFilter` → Needs specification (format, validation, application point) |
| 118 | +- `string[] Fields` → Change to `ViewField[]` for per-field config per @myieye feedback |
| 119 | +- `WritingSystemId[] Vernacular/Analysis` → Types undefined; define as record struct |
| 120 | +- `ViewBase Base` → Enum undefined; use string for extensibility |
| 121 | +
|
| 122 | +### ➕ Consider Adding (3) |
| 123 | +- `Description`, `CreatedBy/CreatedAt`, `IsSystemView` for auditing/access control |
| 124 | +
|
| 125 | +## Blocking Questions |
| 126 | +
|
| 127 | +1. Default mechanism: per-project boolean vs per-role? |
| 128 | +2. Filter format and validation requirements? |
| 129 | +3. Access control model? |
| 130 | +
|
| 131 | +See CUSTOM_VIEW_SUMMARY.md for quick reference or CUSTOM_VIEW_MODEL_ANALYSIS.md for comprehensive details. |
| 132 | +``` |
| 133 | + |
| 134 | +## Related Issues |
| 135 | + |
| 136 | +- [#1050 - Allow setting default View](https://github.com/sillsdev/languageforge-lexbox/issues/1050) |
| 137 | +- [#1049 - Allow creating custom views/layouts](https://github.com/sillsdev/languageforge-lexbox/issues/1049) |
| 138 | +- [#1985 - Custom view data model](https://github.com/sillsdev/languageforge-lexbox/issues/1985) |
| 139 | + |
| 140 | +## Notes |
| 141 | + |
| 142 | +- This corrects the mistake of creating the analysis in the harmony repository |
| 143 | +- The harmony PR will show the files being removed with a redirect notice |
| 144 | +- Both repositories now have appropriate content |
0 commit comments