empirical-org / Empirical-Core

Empirical Core is the mono repo for the Quill.org codebase. This repo contains both the Quill Learning Tools, such as Quill Connect, and the Quill Learning Management System, the platform that enables teachers to provide assignments and view results. (Empirical is the name of the nonprofit building Quill.org).
http://www.quill.org/
GNU Affero General Public License v3.0
25 stars 4 forks source link

individual dataset page #12486

Closed emilia-friedberg closed 6 days ago

emilia-friedberg commented 1 week ago

WHAT

Build the individual dataset page in the Evidence CMS, which displays data subsets, trials, guidelines, and related texts, and includes modal functionality for creating and updating the latter two record types.

WHY

This is the second part of the main project to pull Brendan's work into the main Evidence CMS.

HOW

Build out requisite frontend components and controller actions for all flows on this page. Bonus:

I know this PR is too big. A lot of it is snapshot tests and code reorganization, but I will do a better job in future PRs for this project of splitting out work along logical divisions.

Screenshots

Screenshot 2024-10-28 at 4 09 15 PM Screenshot 2024-10-28 at 4 09 23 PM Screenshot 2024-10-28 at 4 09 32 PM Screenshot 2024-10-28 at 4 10 54 PM Screenshot 2024-10-28 at 4 11 46 PM Screenshot 2024-10-28 at 4 15 06 PM Screenshot 2024-10-28 at 4 15 19 PM Screenshot 2024-10-28 at 4 15 28 PM Screenshot 2024-10-28 at 4 15 39 PM

Notion Card Links

https://www.notion.so/quill/Product-Board-30f97e4eb01246dbb8264253fb385073?p=129d42e6f9418054bb1bd8824be496f1&pm=c

What have you done to QA this feature?

Tested all flows on staging, having Jamie QA as well.

PR Checklist Your Answer
Have you added and/or updated tests? YES
Have you deployed to Staging? YES
Self-Review: Have you done an initial self-review of the code below on Github? YES
brendanshean commented 1 week ago

Screenshot 2024-10-29 at 11 51 35 AM Normally, a large diff is mainly snapshot test or package.lock-json inflation, but this is a lot of code! A significant undertaking.

brendanshean commented 1 week ago

I think this looks PR is amazing. There's one change I'm going to recommend, but I wanted to make sure it was straightforward so I just wrote it up in a branch https://github.com/empirical-org/Empirical-Core/commit/1811426910b80287e13dbf5294f16919eb1753f0

Basically, I think you can simplify the controller create/update with nested attributes.