openfoodnetwork icon indicating copy to clipboard operation
openfoodnetwork copied to clipboard

Convert enterprises/edit page to stimulus

Open binarygit opened this issue 2 years ago • 8 comments

What? Why?

Replaces the angular code that handled rendering of admin/enterprises/<enterprise_name>/edit More context here: https://openfoodnetwork.slack.com/archives/C03QBDHDCSY/p1658325112405699 and #9478

Closes #

What should we test?

  • Visit page admin/enterprises/<enterprise_name>/edit
  • Ascertain only the selected side-bar tab's form is displayed
  • Ascertain that the displayed form changes accordingly when you select another tab

Release notes

Changelog Category: Technical changes

The title of the pull request will be included in the release notes.

Dependencies

Documentation updates

binarygit avatar Aug 03 '22 11:08 binarygit

Some specs are failing. Back into In Dev

jibees avatar Aug 05 '22 09:08 jibees

Hello @jibees I know that here it's saying that two tests in system/admin/enterprises_spec.rb are failing but they are passing locally. I don't know what to do about this :shrug: green

binarygit avatar Aug 08 '22 12:08 binarygit

@binarygit

The spec fails:

     Failure/Error: is_shop = enterprise.sells != "none"
     
     ActionView::Template::Error:
       undefined method `sells' for nil:NilClass


# ./app/helpers/admin/enterprises_helper.rb:18:in `side_menu_items`

undefined method 'sells' for nil:NilClass means that enterprise is nil then.

It use throw when user click on New Enterprise Group ie. /admin/enterprise_groups/new and @enterprise is nil here (it's logical, because I guess there is no enterprise here, right?)

Does this help?

(FYI, the spec fail locally, and when I go to /admin/enterprise_groups/new I have the error)

jibees avatar Aug 08 '22 12:08 jibees

Ok, now I see, you don't run the right spec ; the one failing is spec/system/admin/enterprise_groups_spec.rb

jibees avatar Aug 08 '22 12:08 jibees

arghhh sorry, Should've looked at the error more carefully! I got confused because I had previous failures in enterprises_spec and when I fixed them and pushed I thought these were also in the same file. Thanks for the help :bow:

binarygit avatar Aug 08 '22 13:08 binarygit

@jibees Finally got the tests to pass. Could you have a look when you have the time, and let me know if any changes are required? Thanks!

binarygit avatar Aug 09 '22 10:08 binarygit

After fetching upstream and running yarn install, the yarn.lock file changes. I don't think this should be happening right? I rebased and pushed btw

yarn

Also getting this when I try to commit after changing any controller files, does this mean I need to run prettier manually? I thought it ran automatically when I tried to commit?

 $ git ci --amend
yarn run v1.22.17
warning ../../package.json: No license field
$ pretty-quick --check --staged
🔍  Finding changed files since git revision cb6faed1d.
🎯  Found 1 changed file.
⛔️  Check failed: app/webpacker/controllers/side_menu_controller.js
✗ Code style issues found in the above file(s). Forgot to run Prettier?
error Command failed with exit code 1.
info Visit https://yarnpkg.com/en/docs/cli/run for documentation about this command.
husky - pre-commit hook exited with code 1 (error)

It lets me commit after I run prettier manually, but thought I would ask if this is intentional :smile:

binarygit avatar Aug 09 '22 14:08 binarygit

  • Please, revert your modifications on yarn.lock I don't think they are useful in the context of this PR. I'll make a PR if any errors occurs on yarn.lock. Created: https://github.com/openfoodfoundation/openfoodnetwork/pull/9544

  • No the precommit hook only check (via pretty-quick --check --staged) if any errors occurs. You should run prettier on your modified files via yarn prettier command)

jibees avatar Aug 09 '22 15:08 jibees

Hey @binarygit,

Thanks for this huge piece of work and your detailed notes on what to test. Going through them:

On the admin/enterprises/<enterprise_name>/edit page:

  • Ascertain only the selected side-bar tab's form is displayed -> OK
  • Ascertain that the displayed form changes accordingly when you select another tab -> OK

Uploading Screencast from 17-08-2022 18:04:06.webm…

Had a look as well on other browsers and mobile:

  • Latest Chrome (Ubuntu 22.04)
  • Android and iPhone (Browserstack)

Good to go :tada:

filipefurtad0 avatar Aug 17 '22 17:08 filipefurtad0