USDAForestService / fs-open-forest-platform

Open Forest: The code for an online permitting platform for the U.S. Forest Service.
https://openforest.fs.usda.gov
Other
38 stars 19 forks source link

As a developer, I should be able to change the season dates when I am developing locally. #346

Closed lauraGgit closed 5 years ago

lauraGgit commented 6 years ago

Notes

This is not yet an issue on staging, but appears to multiple developers. When they change an ARP season date it does not show up on the ARP's page, and the ability to buy the permit does not appear. This does seem to work for other forests.

Acceptance Criteria

Tasks

Definition of Done

msadakFS commented 6 years ago

As noted in Slack, I am able to change ARP season date and the buy function appears to work.

lauraGgit commented 6 years ago

@mtlaney are you able to take a look at this one?

mtlaney commented 6 years ago

@lauraGgit sure thing!

mtlaney commented 6 years ago

@lauraGgit Here's where we are. I can get to successfully POST to /admin/christmas-trees/forests/1

screen shot 2018-09-18 at 12 30 59 pm

It will also successfully update my local DB:

screen shot 2018-09-18 at 12 32 07 pm

However locally it looks like the data is being fed by 01-christmasTreesForests.js from the seeders in server, and am unable to update.

Any chance it could be blocked somewhere by the forced Request a Content Change redirect locally that I'm not seeing?

lauraGgit commented 6 years ago

The seeders should only run when you start up the server i.e. if you use npm run dev - so i'm not sure why that would be be conflict.

On Tue, Sep 18, 2018 at 12:37 PM M. Laney notifications@github.com wrote:

@lauraGgit https://github.com/lauraGgit Here's where we are. I can get to successfully POST to /admin/christmas-trees/forests/1 [image: screen shot 2018-09-18 at 12 30 59 pm] https://user-images.githubusercontent.com/18233188/45702031-babb3e80-bb3e-11e8-984a-d02c59a4d92f.png

It will also successfully update my local DB: [image: screen shot 2018-09-18 at 12 32 07 pm] https://user-images.githubusercontent.com/18233188/45702091-e50cfc00-bb3e-11e8-8ca5-62e566b98c4b.png

However locally it looks like the data is being fed by 01-christmasTreesForests.js from the seeders in server, and am unable to update.

Any chance it could be blocked somewhere by the forced Request a Content Change redirect locally that I'm not seeing?

— You are receiving this because you were mentioned. Reply to this email directly, view it on GitHub https://github.com/18F/fs-open-forest-platform/issues/346#issuecomment-422462781, or mute the thread https://github.com/notifications/unsubscribe-auth/AFkgXVU67U5XBrnlprybhYs9quBuY3zPks5ucSFXgaJpZM4We82r .

-- Laura Gerhardt 18F acquisition | GSA | 202-257-7082

lauraGgit commented 5 years ago

@adborden would you be able to take a look at this?

lauraGgit commented 5 years ago

on the server it looks like its this controller: https://github.com/18F/fs-open-forest-platform/blob/dev/server/src/controllers/christmas-tree-admin.es6#L157

and on the frontend: https://github.com/18F/fs-open-forest-platform/blob/dev/frontend/src/app/trees/admin/season-dates/season-dates.component.ts#L112

adborden commented 5 years ago

:eyes:

adborden commented 5 years ago

Surprise! We're mocking it out in local/CI. A nasty surprise in my opinion. Do you know if it's necessary? I'd like to take it out. Otherwise, make it less likely to trip up developers.

https://github.com/18F/fs-open-forest-platform/blob/77236d2f9ca97ad7a2b3374d90698a2eb6f5966a/server/src/models/christmas-trees-forests.es6#L110-L149

lauraGgit commented 5 years ago

Oooooh! I think this was because the admin ability to change the dates was a later feature add, so there was some way to open up a season more easily when you were working development and also in CI.

I think we can go ahead and rip that, but we'll likely have to update all the e2e xmas tests so that all the tests that run against the xmas tree work flow have open seasons.

adborden commented 5 years ago

+1 I'll remove it and update the tests.

adborden commented 5 years ago

I have one more PR coming on this one that will give us a separate test database.

lauraGgit commented 5 years ago

@adborden looks like the change to parse out the database url from the vcap didn't work for the sequelize migrations during deployment. We might want to revert how the database url gets parsed in ./server/.sequelize.js