The values for the mentioned projects filter are stored at project_ids_filter field, we should remove any entries from projects that do not belong to the group hierarchy when calling this method.
The following discussion from !136252 (merged) should be addressed:
While writing this spec I realized that we should probably have a follow-up to add another validation to ValueStreams::Setting#project_ids_filter checking if it belongs to a group value stream.
It does not make much sense to have project_ids_filter on project-level Value Streams. WDYT?
0 of 1 checklist item completed
Designs
Child items
...
Show closed items
Linked items
0
Link issues together to show that they're related or that one is blocking others.
Learn more.
@cablett / @felipe_artur do either of you have capacity to look into this during the remainder of %16.7 or should I consider this as roll over for %16.8?
Should we clean existing invalid values from existing records?
If non-group projects appear in this filter, should we error out (noisy) or silently remove them? I would think the latter might be confusing. Would users expect to know which ones were removed?
The case of 2. is that if there is an error, we'd do so after authorisation so we don't leak which project IDs are in a group.
We don't have strict consistency (FK) for the column so invalid records might appear any time (project is moved / deleted etc). I don't see the point of cleaning up the records at the moment, we could just filter them out.
Maybe as a follow-up we could do some sort of periodical cleanup if necessary.
don't see the point of cleaning up the records at the moment, we could just filter them out.
@ahegyi Yes, my idea was to just filter them out on the fly. We also need to take care to not cause an N+1 queries issue on GraphQL when project_ids_filter is added.
We could also do this in conjunction with #434220.
Note I left on !136252 (merged) - We might also want to consider if the project filter only includes (say) projects not in that group, we can early return an empty set with a message? Do we handle this sort of thing already within VSA/VSD/any other groupoptimize offering?
This feature issue does not have the documentation label.
Please add it if appropriate, because documentation is one of the aspects of our MR acceptance checklist.
This feature issue does not have the documentation label.
Please add it if appropriate, because documentation is one of the aspects of our MR acceptance checklist.
I am closing this directly because I think there is no way to verify this in production. It would need to save the filter value directly into production console which is not ideal.