[BUGFIX] Fix SubscriberList to Subscriber relationship definition by …

[bizmate]

…adding changing invertedBy to mappedBy in Subscriber

Summary

Doctrine bidirectional many to many relationship requires an invertedBy mapping and a mappedBy one.

Fixes validation error as per issue #311

[oliverklee]

Looks good. Would you mind adding the schema validation as a kind of regression test for this?

[bizmate]

@oliverklee I have added the check but it looks like fixing the validation does not fix all problems. For some reasons doctrine still thinks there is a problem. I think it is the double mapping configuration of the many to many bidirectional relation between SubscriberList and Subscriber.

I am surprised this was never seen before. The nicest approach for the project with version 4 is difficult to achieve without automation, ie

• in development always use schema tools (ideally not sql) to set and update schema
• use fixtures for test data instead of sql ( i.e. faker powered fixtures)

As you can see the test is failing. I might check later if I can find a way to make the message
The table with name 'phplist.phplist_listuser' already exists.
go but given my lack of knowledge of the codebase and the manual steps required to run it I might get stuck.
Feel free to let me know your thoughts, also if you have a dev channel like slack or gitter

[bizmate]

https://github.com/phpList/core/blob/master/src/Domain/Model/Subscription/Subscription.php#L23

and

https://github.com/phpList/core/blob/master/src/Domain/Model/Messaging/SubscriberList.php#L118

Are conflicting with each other. The table cannot exist for and entity and at the same time as a many to many mapping. Not sure which one to remove. Also given the integration tests are broken it is also not obvious what to do.

[oliverklee]

Are conflicting with each other. The table cannot exist for and entity and at the same time as a many to many mapping. Not sure which one to remove.

In that case, the m:n mapping should be removed (i.e., it should then go through the other model). (I'd like it more though if we could have both.)

Also given the integration tests are broken it is also not obvious what to do.

AFAICT, the tests still are green. Or am I missing something here?
https://travis-ci.org/phpList/core

[oliverklee]

The table with name 'phplist.phplist_listuser' already exists.

That is because currently the phpList 4 core is expected to use an existing phpList 3 database, not create a new database from scratch. (That will come later when phpList 4 is feature-complete enough to be used without phpList 3.)

[oliverklee]

@bizmate By the way, thank you for contributing! I really appreciate your efforts. ❤️

[bizmate]

@oliverklee sorry I was not able to move this further. I wanted to run the travis commands/CI locally and given the repo does not have built in way to do it automatically i was spending time to work how to run the tests correctly on my local machine as I think this might be the reason why I get the errors in the tests. I will try to spend sometime on it ASAP