okfn-brasil / jarbas

🎩 API for information and suspicions about reimbursements by Brazilian congresspeople
https://jarbas.serenata.ai/
296 stars 61 forks source link

Split logic: public admin and dashboard #238

Closed cuducos closed 7 years ago

cuducos commented 7 years ago

This PR refactor the architecture underneath the dashboard. Basically what we had so far was two files (jarbas/dashboard/sites.py and jarbas/dashboard/admin.py) that handled two different logics:

This PR splits this logic in two apps:

The intention is to extract as much logic as possible related public_admin app in order to have it as a general purpose, external, plugable app (maybe even installable via pip).

jtemporal commented 7 years ago

Hey @cuducos could you help me out a little here? I'm hoping for some idea on how to test this functionality-wise. Any ideas?

( I already ran locally and everything seems okay with the usual testing)

cuducos commented 7 years ago

I'm hoping for some idea on how to test this functionality-wise. Any ideas?

Actually this part of Jarbas is not tested in depth within Jarbas (mostly because it's mostly Django configuration). However in terms of the authentication there some tests in the dashboard and in the new public_admin — these suites makes sure, for exemple, no POST request is allowed and some URLs are dropped (URL with edit, delete, login etc.).

In terms of UI/UX there are actually no tests (this is the part that the code is pretty much declarative, merely Django Admin configuration). Maybe integration tests with someting like PhantomJS could be written in the future.

That said this PR adds zero new functionality (it's an architectural change): therefore if running the dashboard still working fine it works.

Is there anything else you had in mind when you mentioned functionality-wise?

cuducos commented 7 years ago

@jtemporal after 94d8f94 is there anything left from code review? LMK because I'm afraid this branch will soon end up in conflicts regarding #252…

anaschwendler commented 7 years ago

@jtemporal is free, I can test this PR, let me just understand what is being made:

  1. Clone the repository:

    $ git clone git@github.com:datasciencebr/jarbas.git
  2. Open the repo folder:

    $ cd jarbas
  3. Checkout to @cuducos branch:

    $ git checkout -b cuducos-public-admin-architecture-refactor origin/cuducos-public-admin-architecture-refactor
  4. Update the branch:

    $ git merge master
  5. Copy the .env file:

    $ cp contrib/.env.sample env
  6. Build and start services:

    $ docker-compose up -d
  7. Create the database and apply the migrations:

    $ docker-compose run --rm django python manage.py migrate
    $ docker-compose run --rm django python manage.py searchvector
  8. Seeding it with sample data:

$ docker-compose run --rm django python manage.py reimbursements /mnt/data/reimbursements_sample.xz
$ docker-compose run --rm django python manage.py companies /mnt/data/companies_sample.xz
$ docker-compose run --rm django python manage.py suspicions /mnt/data/suspicions_sample.xz
$ docker-compose run --rm django python manage.py tweets
  1. That PR split things, as @cuducos said, so besides accessing localhost:8000/dashboard, and checking if everything is fine, don't think anything else to test, for me it looks good :)