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

[12.0][IMP] payment_pagseguro: Parcelamento de cartão de crédito #1916

Merged
merged 11 commits into from
Jun 1, 2022

Conversation

kmmelcher
Copy link
Contributor

@kmmelcher kmmelcher commented May 20, 2022

Nesse PR, nosso time da KMEE adicionou a opção de parcelar compras no cartão de crédito pelo pagseguro.
Issues e próximos passos estão no README.

Alguns screenshots das mudanças:

image
image

Encontramos também um problema no .gitignore, que não está relacionado diretamente a Pagseguro.
Ao tentarmos adicionar uma nova lib javascript dentro de static/lib como está com ! no .gitignore não conseguimos.
Isso acontecia porque todo diretório /lib estava excluído pelo .gitignore. Dessa forma, colocamos um * para que somente
os arquivos dentro dos diretórios /lib fossem excluídos. Com isso, podemos usar o ! para que as libs sejão uma exceção que possa ser adicionada ao git.

Copy link
Member

@rvalyi rvalyi left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@kmmelcher eu acho o codigo do PR ok, mas mexer no .gitignore não me parece uma boa, porque pelo menos nas branches mais recentes esse arquivo acaba sendo padronizado e sincronizado por um bot da OCA. Sera se vc não consegue adicionar seus arquivos de lib com git add -f arquivo? Veja isso https://stackoverflow.com/questions/8006393/force-add-despite-the-gitignore-file

Tb tem examples de tais arquivos em pastas lib dentro da OCA como https://github.com/OCA/web/tree/14.0/web_timeline/static/lib/vis-timeline sem eles ter mexido no .gitignore.

Da para vc tentar com git add --force e tirar essa alteração do .gitignore então por favor?

@rvalyi
Copy link
Member

rvalyi commented May 20, 2022

@kmmelcher outra coisa que é apenas opinião no caso: se eu fosse a KMEE eu faria esse tipo de modulo com licença LGPL e não AGPL. A galera que vai fazer site que vai interagir com qualquer um facilmente vai cair na paranoia que vai ter que liberar o código de todas customizaçoes web Odoo dela por conta da margem de interpretação da AGPL. E vc pode contar com a própria Odoo SA para espalhar esse medo (FUD) para vender a versão Enterprise dela.
E ao mesmo tempo sendo AGPL não protege muito de alguém fazer um modulo payment_seguro_enterprise fechado que teria mais coisas e ate se for seria então fácil quebrar um tal modulo botando outro open source equivalente (porque nao teria muita coisa). Eu comentei um pouco de forma geral sobre o tema aqui #1374
Mas enfim isso fica a critério de vocês que são os autores deste módulo claro.

@rvalyi
Copy link
Member

rvalyi commented May 26, 2022

Ola kmmelcher vc consegue arrumar a questão do .gitignore?

Vc pode fazer um rebase -i 31cce866b3dfe1fdbf378c366bd38eeea4eb35fe por examplo e alterar os commits depois. Ou se vc achar complicado vc cria uma nova branch com git checkout -b 12.0-payment-pagseguro-credit-2 31cce866b3dfe1fdbf378c366bd38eeea4eb35fe e depois refaz os commits seguente usando git cherry-pick sha1 ou manualmente.

Ai depois que tiver tudo certo manda a branch aqui de novo com git push -f kmee 12.0-payment-pagseguro-credit-2:12.0-payment-pagseguro-credit por examplo. Alguma duvida? Vc quer que a gente faça?

@kmmelcher
Copy link
Contributor Author

Ola kmmelcher vc consegue arrumar a questão do .gitignore?

Vc pode fazer um rebase -i 31cce866b3dfe1fdbf378c366bd38eeea4eb35fe por examplo e alterar os commits depois. Ou se vc achar complicado vc cria uma nova branch com git checkout -b 12.0-payment-pagseguro-credit-2 31cce866b3dfe1fdbf378c366bd38eeea4eb35fe e depois refaz os commits seguente usando git cherry-pick sha1 ou manualmente.

Ai depois que tiver tudo certo manda a branch aqui de novo com git push -f kmee 12.0-payment-pagseguro-credit-2:12.0-payment-pagseguro-credit por examplo. Alguma duvida? Vc quer que a gente faça?

Ok, removi a mudança no .gitignore
Como a lib js já estava lá o arquivo permaneceu.
Precisa de fazer um rebase?

@rvalyi
Copy link
Member

rvalyi commented May 27, 2022

Eu acho que não valeu. Deve ter dado na mesma no final.

@kmmelcher
Copy link
Contributor Author

Eu acho que não valeu. Deve ter dado na mesma no final.

O que eu fiz dá na mesma que fazer git add -f arquivo

@kmmelcher
Copy link
Contributor Author

Eu acho que não valeu. Deve ter dado na mesma no final.

O que eu fiz dá na mesma que fazer git add -f arquivo

Eu dei o comando:
git pull -r oca 12.0
Porém não tem nenhuma mudança para que eu faça um commit,
Não sei se eu fiz o comando errado para forçar um rebase.

@kmmelcher
Copy link
Contributor Author

Eu acho que não valeu. Deve ter dado na mesma no final.

O que eu fiz dá na mesma que fazer git add -f arquivo

Eu dei o comando: git pull -r oca 12.0 Porém não tem nenhuma mudança para que eu faça um commit, Não sei se eu fiz o comando errado para forçar um rebase.

@kmmelcher kmmelcher closed this May 27, 2022
@kmmelcher kmmelcher reopened this May 27, 2022
@@ -0,0 +1,48 @@
// Copyright 2022 KMEEli
Copy link
Member

@rvalyi rvalyi May 27, 2022

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@kmmelcher sem importância, mas eu acho que tem um erro aqui "KMEEli"

Copy link
Member

@rvalyi rvalyi left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

OK aprovado parabens pelo código.

Outra coisa que é apenas opinião: vamos dizer ate a v12 era difícil caçar projeto Odoo fazível no Brasil e nisso usar README em inglês para atrair o olhar da empresa multinacional que já usava Odoo fora do Brasil ou já tava ligada da evolução do mercado era uma boa. Mas eu diria que hoje, em tempo de v14+, a gente já conseguiria encaixar o Odoo meio que na metade das empresas do mercado alvo. Ai nisso, eu usaria ate umas 2 linhas de introdução em inglês, mas o restante eu faria em português mesmo, tipo para bater na pesquisa Google do leigo mesmo. (cc @mbcosta o mesmo comentário vale pros módulos de CNAB). No código quando mais inglês melhor ai sim, porque facilmente pode ter programador de fora.

Mas tanto isso como a questão da licença são coisas que podem ser retrabalhadas la na frente, então por mim já ta OK.

@kmmelcher kmmelcher force-pushed the 12.0-payment-pagseguro-credit branch from 0d3eaad to 758786d Compare May 27, 2022 19:05
@OCA-git-bot
Copy link
Contributor

This PR has the approved label and has been created more than 5 days ago. It should therefore be ready to merge by a maintainer (or a PSC member if the concerned addon has no declared maintainer). 🤖

@rvalyi
Copy link
Member

rvalyi commented May 28, 2022

/ocabot merge major

@OCA-git-bot
Copy link
Contributor

This PR looks fantastic, let's merge it!
Prepared branch 12.0-ocabot-merge-pr-1916-by-rvalyi-bump-major, awaiting test results.

OCA-git-bot added a commit that referenced this pull request May 28, 2022
Signed-off-by rvalyi
@rvalyi
Copy link
Member

rvalyi commented Jun 1, 2022

esse merge parece travado... Eu iniciei o merge do #1924 mesmo assim porque me parece mais urgente para poder integrar os PRs ligados a importação de NFe. Depois vamos iniciar esse merge de novo...

@rvalyi
Copy link
Member

rvalyi commented Jun 1, 2022

/ocabot merge major

@OCA-git-bot
Copy link
Contributor

This PR looks fantastic, let's merge it!
Prepared branch 12.0-ocabot-merge-pr-1916-by-rvalyi-bump-major, awaiting test results.

OCA-git-bot added a commit that referenced this pull request Jun 1, 2022
Signed-off-by rvalyi
@OCA-git-bot
Copy link
Contributor

It looks like something changed on 12.0 in the meantime.
Let me try again (no action is required from you).
Prepared branch 12.0-ocabot-merge-pr-1916-by-rvalyi-bump-major, awaiting test results.

@OCA-git-bot OCA-git-bot merged commit 7ec2b03 into OCA:12.0 Jun 1, 2022
@OCA-git-bot
Copy link
Contributor

Congratulations, your PR was merged at ea5fab7. Thanks a lot for contributing to OCA. ❤️

@marcelsavegnago
Copy link
Member

marcelsavegnago commented Jun 1, 2022

@kmmelcher parabens pelo trabalho.... se quiser portar isso para a 14 temos uma PR de migração em aberto.. Basta submeter uma PR para o nosso fork. Se tiver um tempinho e puder ajudar no teste da 14.0 eu agradeço tbm apesar de que já tem ajudado bastante.

cc @Eduardolimr

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

6 participants