Update list application query for migration task

[khanhtc1202]

What this PR does / why we need it:

Which issue(s) this PR fixes:

Fixes #

Does this PR introduce a user-facing change?:

NONE

[nghialv]

@khanhtc1202 Reuse this pattern to avoid adding a new index. Btw, using created_at is more stable than updated_at.
https://github.com/pipe-cd/pipecd/blob/master/pkg/app/ops/insightcollector/applications.go#L62-L79

[khanhtc1202]

@khanhtc1202 Reuse this pattern to avoid adding a new index. Btw, using created_at is more stable than updated_at. https://github.com/pipe-cd/pipecd/blob/master/pkg/app/ops/insightcollector/applications.go#L62-L79

Ah nice point 👍 Thanks for the comment, but I still think we need to create CreatedAt_Id index in that case, since the code snippet you mentioned above works due to this index is existed
https://github.com/pipe-cd/pipecd/blob/master/pkg/app/ops/firestoreindexensurer/indexes.json#L5-L21

Let's me change the query and update index for that

[nghialv]

but I still think we need to create CreatedAt_Id index in that case, since the code snippet you mentioned above works due to this index is existed

Why don't we use that index with the same query for this migration task?
Deleted applications don't need to be migrated, right?

[khanhtc1202]

but I still think we need to create CreatedAt_Id index in that case, since the code snippet you mentioned above works due to this index is existed

Why don't we use that index with the same query for this migration task? Deleted applications don't need to be migrated, right?

@nghialv Due to this comment #3767 (comment) The deleted application should be migrated as well (to ensure the database manually changes delete status doesn't cause issues later), that's the current implementation, but happy to hear your thought.
cc @knanao

[nghialv]

but I still think we need to create CreatedAt_Id index in that case, since the code snippet you mentioned above works due to this index is existed

Why don't we use that index with the same query for this migration task? Deleted applications don't need to be migrated, right?

@nghialv Due to this comment #3767 (comment) The deleted application should be migrated as well (to ensure the database manually changes delete status doesn't cause issues later), that's the current implementation, but happy to hear your thought. cc @knanao

I feel that supporting the deleted entries is overdoing. The deleted ones are immutable and should not be queried anymore.
Do you guys find out any cases where the deleted entries need to be unmarshaled?

[knanao]

IMO, There are cases where we need to import logically deleted data, such as when we want to analyze data, and I felt that we should avoid panic in such cases.

[nghialv]

Btw, there are a lot number of deleted applications and it is increasing over time, so if this policy is applied, all migration tasks from new will take a lot of time to do.

[khanhtc1202]

since there is no validation for PlatformProvider field in Application model, we can expect that there is no panic if some deleted App model be unmarshaled for now. But we may avoid that by backup datastore and remove (hard delete) deleted Application if need. wdyt?

[nghialv]

since there is no validation for PlatformProvider field in Application model, we can expect that there is no panic if some deleted App model be unmarshaled for now. But we may avoid that by backup datastore and remove (hard delete) deleted Application if need. wdyt?

I think that looks fine. 👍

[khanhtc1202]

I updated the query to use the existed index, ptal 🙏