propel storage bridge attempt #144
Conversation
|
I wouldn't mind finishing this off at the start of next week. Is it possible to make a PR? |
|
Also, what stage is the current code at? Does it just need reviewing, tests and docs doing or is it partially finished? |
As far as I know it worked for @jkabat. I've never had a chance to run it.
Yeap I think you can fork a @jkabat's branch and add some more commits there. After there are two ways to go:
|
|
|
||
| use Payum\Core\Bridge\Propel\Model\om\BaseNotificationDetails; | ||
|
|
||
| class NotificationDetails extends BaseNotificationDetails implements \ArrayAccess, \IteratorAggregate |
There was a problem hiding this comment.
I don think we should have custom models like NotificationDetails, PaymentDetails. Just a basic one like ArrayObject. User can extend it in their code and give it meaningful name
|
I've done a quick review, the solution looks a bit wordy. 7000 thousand lines of code.... |
|
I've made a fork of the pull request and I will have a look Monday and see if I can make any improvements ect.
|
|
I've done this PR quite a long time ago with limited knowledge of collaborating on such a thing. The most of the code is generated by propel:build:model command. There are 2 possibilities:
I think the best thing to do would be new PR based on the recent code. And I could also help to work on it or with writing a tests. My current storage model is based on v13. |
|
I'd prefer to have only a storage. And schema for models If models could be |
|
I will make new fork and new PR during weekend unless @liamsorsby will be faster. |
|
@jkabat I've not got access over this weekend however I am needing this feature for a new project in working on so if you would like me to help with this then I would be more than happy to help. |
|
@liamsorsby @makasim I spent quite time today and here's what i've got: [*] based on following doc I've created custom storage
[*] based on following doc I've created an application with payment controller and propel entities
Config file is created just for easy testing purposes, maybe @makasim could point me how should I pass class path to custom storage - I know I shouldn't have individual custom storage for every model. By calling a "prepare" route are both models correctly persisted to database, but request throw following error at me: It's too late to debug, but I'd like to know if this is correct way of implementation. |
|
@jkabat just creating some tests against what you've already implemented. When I got to the TestModelQuery.php file however the schema that has been implemented seems to be different to that used in Doctrine, is there a reason for this? The tests I've done so far are just the same tests performed with the doctrine test with slight edits to correspond with the difference in the models. For example, propel doesn't use the ObjectManagerMock. |
|
@liamsorsby no there is no reason, it was just too late when I got to the point I described yesterday. I wanted to make the models persist data first to see if it's the good way of implementation. Because propel is building models only for schemas in Bundle folders, I removed schema.xml from Payum library and left the responsibility on the user. Can you please update propel schema the way it passes the tests? |
|
@jkabat I've not actually edited anything other then creating the tests on the current schema you've used. I can update these and commit to this however I'm not sure they are completely correct. I can send them over for you to look at though. Also, as far as I can see I've emmited 2 tests which were: createObjectManagerMock() as propel doesn't use the object manager. I'll commit these now and send you a PR so you can have a look and let me know. |
|
@jkabat, can you confirm if the fork your using is Payum-propel or Payum as I seem to have created the tests on the fork of your repo Payum not Payum-propel |
|
Also, Is it completely necessary for the scheme to extend the model token for example? As I beleive the model that is used there would just be a very basic model to implement in the schema file. As this is the first time I've actually worked with payum I'm a little embarrassed to ask how to test this without actually using symfony or is it a simple index file at the base of the src to test or are you importing your fork via composer into symfony and working from that?
|
|
@liamsorsby I have rebased my fork and updated bridge for payum v0.13. Now we can continue to work on my fork. |
|
Also please check my gists - code in my fork was very outdated. |
|
New PR with branch is here: |
No description provided.