Skip to content

Bug: Existing Topics w/ Parent marked IsDirty() #53

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.

Sign up for GitHub

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

Jump to bottom
Closed
JeremyCaney opened this issue Mar 12, 2021 · 1 comment
Closed

Bug: Existing Topics w/ Parent marked IsDirty() #53

JeremyCaney opened this issue Mar 12, 2021 · 1 comment
Assignees
@JeremyCaney
Labels
Area: Entity Relates to the core data data structure for modeling topic entities. Area: Repositories Relates to the `ITopicRepository` interface or one of its implementations. Priority: 1 Severity 1: Minor Status 5: Complete Task is considered complete, and ready for deployment. Type: Bug Behavior that is inconsistent with documented or expected behavior.
Milestone
OnTopic 5.1.0

Comments

@JeremyCaney
Copy link
Member

JeremyCaney commented Mar 12, 2021 •
edited
Loading

When an existing topic is loaded (i.e., a topic with an Id), they should be not be marked as IsDirty(). This works if a new Topic has its Key or ContentType set (both of which are required parameters). If the Parent property is set, however, then the Topic is being reported as IsDirty(). This means it will saved as part of a recursive Save() even if no other changes have been made, which can have a significant performance impact on recursive saves, such as imports using the OnTopic Data Transfer library.
@JeremyCaney JeremyCaney added Type: Bug Behavior that is inconsistent with documented or expected behavior. Area: Repositories Relates to the `ITopicRepository` interface or one of its implementations. Area: Entity Relates to the core data data structure for modeling topic entities. Severity 1: Minor Priority: 1 Status 5: Complete Task is considered complete, and ready for deployment. labels Mar 12, 2021
@JeremyCaney JeremyCaney added this to the OnTopic 5.1.0 milestone Mar 12, 2021
@JeremyCaney JeremyCaney self-assigned this Mar 12, 2021
@JeremyCaney
Copy link
Member Author

This issue is resolved by calling MarkClean() if IsNew in the constructor (affecba). The fix is validated by a new unit test (c0955ab).

JeremyCaney added a commit that referenced this issue Mar 13, 2021
@JeremyCaney
Merge branch 'maintenance/unit-test-coverage' into develop
58dec57 
Went through Azure DevOps' code coverage report, and used it to patch gaps in our unit tests. There remain a lot of gaps in the `OnTopic.Data.Sql` and the `OnTopic.AspNetCore.Mvc` libraries, which will be better served by integration tests. Outside of those two libraries, we're now above 99% code coverage on the main libraries.

As part of this expanded coverage, I identified and resolved a number of bugs that hadn't yet been identified, particularly around the `ITopicRepository` implementations. These include:
- `SqlTopicRepository.Load()` won't delete attributes (#45) or topic references (#46)
- `SqlTopicRepository.Load()` treats `Topic.References` as `!IsFullyLoaded` if there are any deleted references (#46)
- `TopicRepository.Load(topic, version)` validates `version` against `DateTime.Now` not `DateTime.UtcNow` (#56)
- `TopicRepository.Delete()` throws exception if the expected `ReturnCode` is missing (#48)
- `TopicRepositoryDecorator.Delete()` didn't set default value for the `isRecursive` parameter (#49)
- `TopicRepository.TopicRenamed` event not being raised on `Save()` (#50)
- `Topic.IncomingRelationships` aren't being properly cleared by calls to `TopicReferenceCollection.Clear()` (#51) and `SetValue()` (#52)
- `Topic.IsDirty()` is being set to `true` if `Topic.Parent` is set during construction, even if it's an existing entity (#53)
- `ReadOnlyKeyedTopicCollection` constructor requires nullable, optional `innerCollection` parameter (#54)
- `ContentTypeDescriptor.IsTypeOf()` always returns `true` and thus is non-functional (#55)
- `TrackedRecord<T>` and derivative constructors don't accept a null `value`, even though the corresponding `Value` property does (#59)

In addition, it identified two bugs which are scheduled for OnTopic 5.1.0, but which haven't yet been resolved:
- `TopicRepository.GetAttributes()` returns mismatched attributes for both `isDirty` and `!isDirty` (#44)
- `SqlTopicRepository.TopicLoaded` event is not raised with the `version` parameter where appropriate (#58)
@JeremyCaney JeremyCaney changed the title Bug: Existing Topics w/ Parent marked IsDirty() Bug: Existing Topics w/ Parent marked IsDirty() Mar 29, 2021
@JeremyCaney JeremyCaney mentioned this issue Apr 7, 2021
Merged
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Assignees

@JeremyCaney JeremyCaney

Labels
Area: Entity Relates to the core data data structure for modeling topic entities. Area: Repositories Relates to the `ITopicRepository` interface or one of its implementations. Priority: 1 Severity 1: Minor Status 5: Complete Task is considered complete, and ready for deployment. Type: Bug Behavior that is inconsistent with documented or expected behavior.
Projects
None yet
Milestone
OnTopic 5.1.0
Development

No branches or pull requests
1 participant
@JeremyCaney