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

Aligning with changes in BHoMAdapter Refactoring Level 04 #11

Merged
merged 2 commits into from
Jan 24, 2020

Conversation

RasmusCanfjorden
Copy link
Contributor

NOTE: Depends on

BHoM/BHoM_Engine#1371
BHoM/BHoM_Adapter#164
BHoM/BHoM_UI#167

Issues addressed by this PR

Fixes #3

Test files

@RasmusCanfjorden RasmusCanfjorden added status:do-not-merge For instance, test PR, requires further discussion, or dependant PRs not ready for merge type:external-api-changes Imposed changes, including from dependency across other BHoM repos labels Jan 13, 2020
@RasmusCanfjorden RasmusCanfjorden self-assigned this Jan 13, 2020
Comment on lines 18 to 28
// If unset, set the pushType to AdapterSettings' value (base AdapterSettings default is FullCRUD).
if (pushType == PushType.AdapterDefault)
pushType = m_AdapterSettings.DefaultPushType;

IEnumerable<IBHoMObject> objectsToPush = ProcessObjectsForPush(objects, actionConfig); // Note: default Push only supports IBHoMObjects.

bool success = true;

CreateModel(objectsToPush.ToList());

return success ? objects.ToList() : new List<object>();
Copy link
Member

Choose a reason for hiding this comment

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

I see what you want is to simply have the Push call Create, which is the only CRUD method implemented.

This can be achieved by setting the m_AdapterSetting private field in your constructor. Please add this in the Adapter constructor:

m_AdapterSettings.DefaultPushType = oM.Adapter.PushType.CreateOnly;

This way the Adapter will simply call ICreate method.
If in the future you will write the IRead or any other CRUD method, you can switch this to oM.Adapter.PushType.FullCRUD or anything you need.

Additionally, as I see there's no DependencyTypes in the Adapter, please also add below the previous:

m_AdapterSettings.HandleDependencies = false;

Then please delete the Push override completely, and test if all is good.

@alelom alelom self-requested a review January 22, 2020 15:10
@alelom
Copy link
Member

alelom commented Jan 22, 2020

Approved on the basis of the new changes I put in.
This needs testing. As discussed, no testing can be done from my side on this.

Please test this thoroughly and leave a request for changes in case this does not work under the script tests.

@FraserGreenroyd @MajaLindroth

Copy link
Contributor

@FraserGreenroyd FraserGreenroyd left a comment

Choose a reason for hiding this comment

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

LGTM

@FraserGreenroyd FraserGreenroyd merged commit ee7ddc0 into master Jan 24, 2020
@FraserGreenroyd FraserGreenroyd deleted the BHoM_Adapter-refactoringLvl04-01 branch January 24, 2020 12:09
@FraserGreenroyd FraserGreenroyd removed the status:do-not-merge For instance, test PR, requires further discussion, or dependant PRs not ready for merge label Jan 24, 2020
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
type:external-api-changes Imposed changes, including from dependency across other BHoM repos
Projects
None yet
Development

Successfully merging this pull request may close these issues.

OpenStudio_Toolkit: Align to changes for BHoM_Adapter refactoring level 04
3 participants