Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Remove v1 profile page and related code #211

Closed
kevinrobinson opened this issue Mar 18, 2016 · 4 comments
Closed

Remove v1 profile page and related code #211

kevinrobinson opened this issue Mar 18, 2016 · 4 comments

Comments

@kevinrobinson
Copy link
Contributor

This is part of the cleanup for #5.

We should wait a week or two until after v2 is fully rolled out and stable. Especially with adding new bits to the v2 profile page like DIBELS and ACCESS, it's useful to have the v1 page still around for quick validation.

There's also some CSV export logic and really good authorization tests that are written for the v1 profile page, so should move those over to the new profile page too.

Ultimately, this means we remove the redirect that's there now, and the profile action becomes the show action.

@kevinrobinson kevinrobinson added this to the Kevin + Alex Spring Project Plan milestone Mar 18, 2016
@kevinrobinson kevinrobinson modified the milestones: Kevin + Alex Spring Project Plan, 1. Profile page v2 Apr 5, 2016
alexsoble added a commit that referenced this issue Apr 5, 2016
alexsoble added a commit that referenced this issue Apr 5, 2016
+ Not important anymore because Profile V2 CSV export is done completely client-side
+ #211
alexsoble added a commit that referenced this issue Apr 5, 2016
+ Authorization specs remain the same
+ No need to test server-side CSV rendering
+ Test assignment of student serialized data
+ #211
@alexsoble
Copy link
Member

@kevinrobinson PR #281 covers the controller and view part of this issue.

It looks like there's also a data model / data migration aspect here too.

The StudentNote and ProgressNote models are used in V1 but not V2. We should delete those models (and their corresponding specs, helpers, generators, etc.) as part of this work.

There are 0 StudentNotes and 49 ProgressNotes in the production database. These were added by Jill at MTSS / SST meetings. In order to not lose this data, we'll need a migration that turns ProgressNote data into EventNote data.

@kevinrobinson
Copy link
Contributor Author

Awesome!

Yeah and to clarify I think those models are used in v2, read and then
rendered as 'older note' or something like (although maybe not).

I had been thinking we'd just keep the old models around but I think doing
a full migration sounds like a better way to simplify. 👍
On Tue, Apr 5, 2016 at 11:34 AM Alex Soble notifications@github.com wrote:

@kevinrobinson https://github.com/kevinrobinson PR #281
#281 covers the
controller and view part of this issue.

It looks like there's also a data model / data migration aspect here too.

The StudentNote and ProgressNote models are used in V1 but not V2. We
should delete those models (and their corresponding specs, helpers,
generators, etc.) as part of this work.

There are 0 StudentNotes and 49 ProgressNotes in the production database.
These were added by Jill at MTSS / SST meetings. In order to not lose this
data, we'll need a migration that turns ProgressNote data into EventNote
data.


You are receiving this because you were mentioned.
Reply to this email directly or view it on GitHub
#211 (comment)

@alexsoble
Copy link
Member

Oh yeah, you are correct! I see the "Old progress note" / "Old intervention" bit now, didn't notice before.

In that case I think a full migration is a "nice to do" refactor but not critical. The StudentNote and ProgressNote models are still being exercised by Profile V2 so they are not just cruft, unlike the view / controller code.

@alexsoble
Copy link
Member

alexsoble commented Apr 14, 2016

Almost done here. The only code left in app/assets/javascripts/student is the profile_charts directory. Some of the V1 profile chart code is being used by V2, some is not.

So remaining steps are:

  1. Figure out which profile_charts code is unused, delete that.
  2. Move profile_charts code in use over.
  3. Get rid of the "V2" part of files, components, and folders that have it in their name. Would be a little confusing for a new contributor to see V2 everywhere when there's no more V1.

alexsoble added a commit that referenced this issue Apr 15, 2016
+ These charts are now handled (much more concisely!) by the ProfileChart component
+ #211
alexsoble added a commit that referenced this issue Apr 17, 2016
+ Really more part of #211, missed it the first time around
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

2 participants