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

i-Educar on Laravel #389

Merged
merged 337 commits into from Nov 30, 2018

Conversation

Projects
None yet
@edersoares
Member

edersoares commented Aug 17, 2018

Conforme levantado na issue #217 a intenção deste pull request é adicionar o Laravel ao i-Educar para que seja possível utilizar todos os recursos do framework e manter a retro compatibilidade com o código legado do i-Educar.

O que está incluído

  • Uma instalação limpa do Laravel Framework v5.7.15.
  • Imagens oficiais Docker: nginx, php-fpm:7.2, postgres:9.5, redis.
  • Atualização do readme.
  • Atualização do .editorconfig.
  • Atualização do .gitignore.
  • Remove Phinx em prol do Blueprint.
  • Criado arquivo de configuração da estrutura legada.
  • Arquivos de exemplos de configuração foram atualizados.
  • Comando do Artisan para gerar os links simbólicos necessários para executar o i-Educar corretamente.
  • Rotas para capturar as requisições do i-Educar.
  • Controlador para tratar as requisições do i-Educar.

Arquitetura

O Laravel passará a ser o ponto central de todas as requisições da aplicação. O web server irá passar a request ao Laravel que irá carregar a rota correta do i-Educar, este irá devolver o conteúdo para o Laravel que então irá devolver a response ao web server.

laravel

O i-Educar está literalmente envolvido pelo Laravel.

Desvantagens

  • A aplicação sofrerá uma pequena perda de performance inicialmente até o código do i-Educar ser refatorado.
  • Quando um erro E_RECOVERABLE_ERROR for lançado, a aplicação será interrompida e o erro retornado.
  • Esta branch trará uma quebra de versão arquitetural e de requisitos.

Vantagens

  • Atualização para o PHP 7.2 (a versão 7.0 terá seu suporte encerrado em dezembro deste ano).
  • Suíte de testes pronta e apta para criação de testes unitários, testes de integração e testes de aceitação.
  • Utilização de namespaces nos arquivos legados sem a necessidade de require de arquivos ou do autoload.php.
  • Permitir a criação de novas funcionalidades com melhor tecnologia e maior qualidade mas garantindo o funcionamento e manutenção das funcionalidades legadas.
  • Uso de um framework com versões LTS.
  • Todas as funcionalidades e ecossistema do Laravel estarão disponíveis para uso (também no código legado)

Inovação

Será possível acoplar mais facilmente serviços de terceiros ao i-Educar assim como ferramentas que possam facilitar muito o desenvolvimento e qualidade do software:

  • Tornar o i-Educar um servidor OAuth2 com Passport para que os softwares acoplados nele, possam se autenticar em um único lugar.
  • Controlar os workers que executam os jobs de forma assíncrona com o Horizon.
  • Facilitar a buscar por texto na aplicação e na auditoria através de search engines como Elastic Search ou Algolia usando Scout.
  • Permitir que os usuários façam login no i-Educar usando suas contas do Facebook, LinkedIn, Google ou outro provedor com Socialite.

Ajuda e discussão

Gostaríamos de pedir ajuda para a validação desta arquitetura, assim como abrir espaço para discussão sobre o assunto.

Obrigado.

@farribeiro

This comment has been minimized.

Contributor

farribeiro commented Aug 17, 2018

Isso já é melhor do que nada

Este assunto pode utilizar como upstream/roadmap/milestone v3 ( #322 )

@MarceloCajueiro

This comment has been minimized.

Member

MarceloCajueiro commented Aug 22, 2018

Eu gosto muito da ideia pela facilidade.

Gostaria de pedir um feedback do @diegoholiveira, que tem ajudado nos pensamentos do futuro do i-educar.

@JDias

This comment has been minimized.

JDias commented Aug 22, 2018

eu achei sensacional!

kudos @edersoares !
Como o @farribeiro comentou, a partir desse divisor de aguas é possível se definir milestones como modelar o banco para o eloquent ou integração com o nuxt (que está para sair uma versão nova)
https://github.com/cretueusebiu/laravel-nuxt

o @edersoares realizou bem mais do que eu tinha pensado sequer ser possível e aparentemente a conversa na FISL sintetizou todas minhas inquietações.

@diegoholiveira

This comment has been minimized.

diegoholiveira commented Aug 22, 2018

Sensacional essa solução. Eu gosto bastante do laravel, mas nunca tinha me passado pela cabeça fazer o link com um aplicação legada da forma que vc fez, por isso eu sempre optei pelo symfony pra ir inserindo o framework componente a componente.

Dúvida: o phinx não poderia ser removido em favor do sistema de migrations do eloquent?

Uma sugestão que eu deixo aqui é usar o sistema de packages do Laravel (https://laravel.com/docs/5.6/packages) para desenvolver os módulos da aplicação: ou seja, todo model / controller / view fica dentro de um modulo, de forma que seja possível pra uma instalação habilitar ou não determinados pacotes, isso é legal principalmente pros relatórios. Claro que isso cria uma complexidade que agora seja ruim, no sentido de dificultar um pouco o desenvolvimento, mas acho que o ganho lá na frente pode ser bem legal.

@edersoares

This comment has been minimized.

Member

edersoares commented Aug 22, 2018

Obrigado @JDias, foi um trabalho que partiu de um brainstorm que tivemos no FISL e aproveitamos para colocar em prática.

Após validarmos esta estrutura, acredito que a evolução e refatoração do i-Educar se tornará mais ágil e um pouco mais fácil.

Bacana seu feedback @diegoholiveira.

A sua dúvida quanto ao Phinx é bem pertinente, a intenção é remover o Phinx em prol do Blueprint do Laravel.

Atualmente em meu tempo livre, estou desenvolvendo uma biblioteca para fazer a engenharia reversa do banco atual e criar automaticamente os arquivos de migrations do Laravel, estou estudando fazer a engenharia reversa de views e functions além de migrar a estrutura das tabelas.

Como esta branch é uma quebra de versão, seria o marco zero das migrations da estrutura atual.

Pensamos também em usar o sistema de pacotes do Laravel como você disse, "modularizar" a aplicação e torná-la extensível a medida que o i-Educar for crescendo. A complexidade é algo que será inevitável de qualquer forma, porém com esta estrutura será mais fácil criar e testar os pacotes individualmente e tornar o i-Educar um sistema extensível.

Obrigado por participar da discussão.

Abraço.

@farribeiro

This comment has been minimized.

Contributor

farribeiro commented Aug 22, 2018

Seria mais facil fazer um meeting no IRC em invés de call para essas questões, que tal no ##i-educar na freenode e um calendário de sprints no Google?

@diegoholiveira

This comment has been minimized.

diegoholiveira commented Aug 22, 2018

Atualmente em meu tempo livre, estou desenvolvendo uma biblioteca para fazer a engenharia reversa do banco atual e criar automaticamente os arquivos de migrations do Laravel, estou estudando fazer a engenharia reversa de views e functions além de migrar a estrutura das tabelas.

O que pode ser feito nesse sentido é usar um arquivo .sql com a estrutura atual, e no migrate inicial executar esse arquivo. Ai vc teria um schema-up.sql e um schema-down.sql. Ai no up e down vc executa cada um destes arquivos. As alterações futuras, já parte pra fazer no formato do Laravel. O que acha?

@JDias

This comment has been minimized.

JDias commented Aug 22, 2018

Provavelmente o @edersoares já sabe que já existe conversor de sql para migrations/seeds laravel.

O que não converte para um formato compatível com o eloquent.

Esse é um papo um pouco mais demorado.

Quanto as views eu nem imagino o que tem em mente.😊

Existe alguma expectativa para a disponibilidade do "wrapper" laravel com ieducar?

**Respondendo a mim mesmo** https://github.com/portabilis/i-educar/tree/laravel

Alguem testou o debugabar do laravel (eu particularmente não gosto, mas fiquei curioso) nos requests para o código legado?

Edited

Eu, particularmente não me preocuparia em refazer o banco do i-educar utilizando as migration @edersoares , ao invés disso me dedicaria a modelar um novo banco no "estilo" eloquent (e eu usaria e abusaria dos relacionamentos polimórficos, imageable, adressable, tagable etcable ... ) e ao fim ter uma rotina de importação dos dados do banco de dados legado.

ter a migrations de uma estrutura incompatível não teria sentido pratico, acho @diegoholiveira , o que talvez pudesse ser discutido, e eu não sou capaz, seria utilizar coisas como o repository pattern (que pessoalmente eu não gosto) mas que se aproxima mais (até onde eu entendo) de como o i-educar atualmente se comunica com o banco.

ter "decidido" pelo eloquent já é um belo progresso, fica no ar essa questão do repository pattern, que eu, ignorantemente, sou contra.

aqui tem um texto sobre repository pattern https://code.tutsplus.com/pt/tutorials/the-repository-pattern-in-laravel-5--cms-25464

e aqui o "pacote" laravel - https://github.com/andersao/l5-repository

@edersoares

This comment has been minimized.

Member

edersoares commented Aug 23, 2018

O que pode ser feito nesse sentido é usar um arquivo .sql com a estrutura atual, e no migrate inicial executar esse arquivo. Ai vc teria um schema-up.sql e um schema-down.sql. Ai no up e down vc executa cada um destes arquivos. As alterações futuras, já parte pra fazer no formato do Laravel. O que acha?

Sim, @diegoholiveira , é uma opção, atualmente já é assim com o Phinx e se conseguirmos finalizar a validação da estrutura com o Laravel logo, provavelmente seguiremos neste caminho.

Nós pensamos em fazer engenharia reversa justamente para possibilitar a modularização do i-Educar, onde as migrations ficarão cada qual em seu pacote.

Outro aspecto é que o nosso pensamento é começar a remover gradativamente funções, lógica e regras do banco de dados para tornar o i-Educar independente de banco, assim cada um poderá utilizar a solução de persistência que desejar. Mas são apenas planos no momento.

Eu já encontrei algumas bibliotecas para fazer esta engenharia reversa @JDias, mas elas foram desenvolvidas para bancos de dados mais organizados. Os plano é criar as migrations das estruturas com o intuito de começar a corrigir a modelagem do banco de dados.

@JDias ainda não temos uma data para fundir esta branch a master, irá passar pelo nosso setor de qualidade.

O i-Educar precisa de uma nova modelagem do banco de dados sim, mas não temos como modelar um novo banco e migrar estas informações, vamos fazer isto gradativamente.

Eloquent é um active record diferente de um data mapper como o Doctrine, porém suas funcionalidades são excepcionais e o pretendemos sim utilizar o Repository Pattern (baseado em interfaces) assim como uma camada de serviços, mas isto é assunto para um momento futuro.

@JDias

This comment has been minimized.

JDias commented Aug 23, 2018

@edersoares eu vi comentários em relação ao socialite e ao passport (bem como a ideia de se ter uma camada de serviços), o sr tem alguma opinião formada em relação a utilização de uma API gateway?

@eberfreitas

This comment has been minimized.

Contributor

eberfreitas commented Aug 23, 2018

Pra gente conseguir colocar um ponto aqui... Existe alguém que se opõe a essa direção de utilizarmos o Laravel? Se não, já poderíamos ir tocando mais o barco logo :D

@munizeverton

This comment has been minimized.

Contributor

munizeverton commented Aug 23, 2018

Excelente @edersoares !

Realmente no início eu tinha a ideia de ter algo mais modularizado, pra dar a possibilidade de fazer a migração aos poucos.
Mas do jeito que você fez nós temos o melhor dos dois mundos. Podemos fazer a migração aos poucos mas também já temos o código legado dentro da estrutura de um framework, trazendo para nós todos os benefícios de usar uma ferramenta desse tipo.

Como o pessoal já falou aí, a ideia a ir migrando aos poucos a forma como as coisas são feitas no código legado, pra dentro do Laravel.

Aqui eu fiz um exemplo simples de como poderíamos usar coisas do framework, já no código legado

captura de tela 2018-08-23 as 18 54 46
Laravel debugbar no i-Educar

Concordo também com a ideia de ir transformando tudo em packages.

No mais, é isso. Parabéns e vamos em frente!

@giustin

This comment has been minimized.

Member

giustin commented Aug 23, 2018

Fala @edersoares !

Cara, parabéns! Excelente abordagem. Com isso conseguimos estabelecer uma visão clara de produto e definir o que o software resolve, pra quem, seu roadmap de crescimento e o que poderá ser resolvido e expandido por módulos ou soluções de terceiros conforme necessidades específicas. Atualmente, sem pensar nesta evolução que você está propondo, incluir novas features é aumentar o tamanho do problema.

Vamos em frente!

@vitormattos

This comment has been minimized.

Contributor

vitormattos commented Aug 27, 2018

Creio que seja uma mudança muito impactante para se fazer sem uma boa cobertura de testes na aplicação. Atualmente o sistema tem apenas 7 asserts nos testes e mesmo assim é preciso mover arquivos para que os 7 testes passem.

É inviável fazer um code review em um PR tão grante como este, o que aumenta ainda mais o risco de quebrar algo no projeto.

De mais, é interessante a inclusão de um framework no projeto.

@farribeiro

This comment has been minimized.

Contributor

farribeiro commented Aug 28, 2018

Que tal fazer um novo branch tipo v3.0.0 e refazer o review do código em etapas menores?

@vitormattos

This comment has been minimized.

Contributor

vitormattos commented Aug 28, 2018

@farribeiro é uma boa sugestão mas não resolve o problema maior que é a falta de cobertura de testes para se ter um mínimo de segurança de que tudo continuará funcionando após o PR.

O Travis está sendo utilizado apenas para fazer build para o site e o Scrutinizer para análise de código que não envolve testes unitários ou funcionais que é o essencial para garantir o funcionamento do código.

@edersoares

This comment has been minimized.

Member

edersoares commented Aug 28, 2018

Hoje não há cobertura de testes no i-Educar, os testes que existiam foram removidos devido a não terem evoluído junto com o código atual.

Esta mudança em si, não irá modificar muito a estrutura atual do i-Educar, apenas passará a requisição por dentro do Laravel.

O maior problema que encontramos até agora, foi devido a atualização de versão do PHP que na 7.1 houve quebra de compatibilidade conforme link http://php.net/manual/en/migration71.incompatible.php:

Throw on passing too few function arguments

Previously, a warning would be emitted for invoking user-defined functions with too few arguments. Now, this warning has been promoted to an Error exception. This change only applies to user-defined functions, not internal functions. For example:

<?php
function test($param){}
test();

The above example will output something similar to:

Fatal error: Uncaught ArgumentCountError: Too few arguments to function test(), 0 passed in %s on line %d and exactly 1 expected in %s:%d

Funções definidas pelo usuário que sejam chamadas informando um número menor de parâmetros do que os obrigatórios, ao invés de emitir um warning, uma Error exception será lançada, o que acaba quebrando a aplicação.

A questão da versão da aplicação, ainda estamos estudando a ideia de iniciar um 3.0.0 ou simplesmente evoluir para 2.1.0, pois pensamos que uma versão 3 seria o i-Educar com um padrão e conceitos já bem definidos.

Em todo caso, nosso setor de qualidade também irá testar toda a aplicação. Estamos trabalhando na branch laravel-dusk com testes de aceitação para garantir que as páginas estão sendo exibidas para o usuário. Queremos garantir seu funcionamento e a intenção é que a comunidade ajude na validação do mesmo fazendo testes funcionais ou criando testes automatizados.

@JDias

This comment has been minimized.

JDias commented Aug 28, 2018

O que vc fez @edersoares deveria ter um projeto a parte chamado "laraveling legacy systems" 👍😊

@vitormattos

This comment has been minimized.

Contributor

vitormattos commented Aug 28, 2018

Puxa! corrigiram os testes eliminando os testes? :'(

@farribeiro

This comment has been minimized.

Contributor

farribeiro commented Aug 28, 2018

@vitormattos Revert neles!

@edersoares

This comment has been minimized.

Member

edersoares commented Aug 28, 2018

@vitormattos entendemos que usar esforços para corrigir testes defasados que não tinham manutenção a anos não seria interessante.

Mas caso você queira utilizar algum tempo para corrigi-los, volte a release 2.0.0, lá tem os testes antigos do i-Educar, fique a vontade para corrigi-los e abrir um PR :)

@vitormattos

This comment has been minimized.

Contributor

vitormattos commented Sep 3, 2018

@edersoares testes restaurados, mocks de banco não estão mais funcionando e foram ignorados. Mandei um pr #402

eberfreitas and others added some commits Nov 30, 2018

@edersoares edersoares force-pushed the laravel branch from f3d1868 to a92797f Nov 30, 2018

@edersoares edersoares force-pushed the laravel branch from 07ed749 to eca6160 Nov 30, 2018

eberfreitas and others added some commits Nov 30, 2018

Merge pull request #4566 from portabilis/issue-4558
Script para criação de builds de release
Merge pull request #4573 from portabilis/issue-4559
Interface de instalação do i-Educar

@edersoares edersoares force-pushed the laravel branch from 9233cc6 to 3853047 Nov 30, 2018

@edersoares edersoares merged commit 8eb786e into master Nov 30, 2018

1 check passed

Scrutinizer Analysis: 4376 Issues, 3183 Patches – Tests: passed
Details
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment