Skip to content

Commit

Permalink
We do need to actually validate the head entities, can't skip them
Browse files Browse the repository at this point in the history
re: the fix for #8632 - we can't actually skip validation on these.
The better solution is to move the check to getIssues() so the user isn't
credited for causing the issues unless it's something they actually touched
  • Loading branch information
bhousel committed Aug 27, 2021
1 parent 113f079 commit 7896107
Showing 1 changed file with 10 additions and 9 deletions.
19 changes: 10 additions & 9 deletions modules/core/validator.js
Original file line number Diff line number Diff line change
Expand Up @@ -189,16 +189,22 @@ export function coreValidator(context) {
let seen = new Set();
let results = [];

// collect head issues - caused by user edits
// collect head issues - present in the user edits
if (_headCache.graph && _headCache.graph !== _baseCache.graph) {
Object.values(_headCache.issuesByIssueID).forEach(issue => {
// In the head cache, only count features that the user is responsible for - #8632
// For example, a user can undo some work and an issue will still present in the
// head graph, but we don't want to credit the user for causing that issue.
const userModified = (issue.entityIds || []).some(id => _completeDiff.hasOwnProperty(id));
if (opts.what === 'edited' && !userModified) return; // present in head but user didn't touch it

if (!filter(issue)) return;
seen.add(issue.id);
results.push(issue);
});
}

// collect base issues - not caused by user edits
// collect base issues - present before user edits
if (opts.what === 'all') {
Object.values(_baseCache.issuesByIssueID).forEach(issue => {
if (!filter(issue)) return;
Expand Down Expand Up @@ -682,11 +688,6 @@ export function coreValidator(context) {
const entity = graph.hasEntity(entityID); // Sanity check: don't validate deleted entities
if (!entity) return;

// In the head cache, only validate features that the user is responsible for - #8632
// For example, a user can undo some work and an issue will still present in the
// head graph, but we don't want to credit the user for causing that issue.
if (cache.which === 'head' && !_completeDiff.hasOwnProperty(entityID)) return;

// detect new issues and update caches
const result = validateEntity(entity, graph);
if (result.provisional) { // provisional result
Expand Down Expand Up @@ -869,7 +870,7 @@ function validationCache(which) {
entityIssueIDs.forEach(issueID => {
const issue = cache.issuesByIssueID[issueID];
if (issue) {
(issue.entityIds || []).forEach(relatedID => result.add(relatedID))
(issue.entityIds || []).forEach(relatedID => result.add(relatedID));
} else { // shouldnt happen, clean up
delete cache.issuesByIssueID[issueID];
}
Expand All @@ -878,7 +879,7 @@ function validationCache(which) {
});

return result;
}
};


return cache;
Expand Down

0 comments on commit 7896107

Please sign in to comment.