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

Delete embedded causes corruption #2

Closed
wants to merge 9 commits into from

Conversation

dgknght
Copy link

@dgknght dgknght commented Dec 16, 2020

This is a follow up to this PR, as we've found more instances where setting the version is causing corrupted documents.

Additionally, in order to get around some line limits that are enforced on the repo, some code has been rearranged. A commit was created for each step in the process with an explanation of the motivation.

Doug Knight added 4 commits December 16, 2020 12:05
The module line limit is preventing extra logic from being added to
ensure that conflicting updates are not being created. I've chosen an
arbitrary method for reducing the line count in the module. I'm open to
suggestions if someone else has a better idea in mind.
The line limit for blocks is prevents additional specs from being added
for Mongoid::History::Trackable. I've picked an arbitrary method of
reducing the size of the block in order to get past that limit. I'm open
to suggestions if someone has a better idea in mind.
After making the change to logic for incrementing the version number
this spec started failing even though the behavior was actually correct.
Though post.history_tracks returned an updated list of records, the
call to #last continued to return the same "destroy" track, causing the
comment to be created twice, instead of being recreated and then
redestroyed. Addressing the last record by index appears to have
resolved the problem.
Copy link

@cgriego cgriego left a comment

Choose a reason for hiding this comment

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

Instead of splitting up the module and specs arbitrarily, let's just raise the limits in the .rubocop_todo.yml file.

The functional changes look good.

@pull-request-size pull-request-size bot added size/M and removed size/L labels Dec 17, 2020
@dgknght
Copy link
Author

dgknght commented Dec 17, 2020

@cgriego, That makes the PR much smaller, but it does beg the question, why have the constraint in the first place? :)

@dgknght dgknght requested a review from cgriego December 17, 2020 16:53
Copy link

@cgriego cgriego left a comment

Choose a reason for hiding this comment

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

It does, but that's also why it's in a "todo" file. 😉

@dgknght
Copy link
Author

dgknght commented Jan 5, 2021

Superceded by this PR

@dgknght dgknght closed this Jan 5, 2021
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants