Add Bug-2062665 [Suggest] Add new suggestionId field on Suggestion.amp

[mashalifshin]

📜 Tickets

Jira ticket

💡 Description

This PR is to support an application-services suggest rust component change that adds a new suggestion_id field to the sponsored suggestion, for improved business telemetry.

Once this is in place, the plan is to send back the suggestion_id field with the fx-suggest glean ping across all clients for online and offline modes (Desktop online quicksuggest has already recently added this (glean dictionary link), working on the rest).

See the application-services PR here

🎥 Demos

This is an internal change that doesn't surface in the app yet.

📝 Checklist

• I filled in the ticket numbers and a description of my work
• I updated the PR name to follow our PR naming guidelines
• I ensured unit tests pass and wrote tests for new code
• [] If working on UI, I checked and implemented accessibility (Dynamic Text and VoiceOver)
• [] If adding telemetry, I read the data stewardship requirements and will request a data review
• [] If adding or modifying strings, I read the guidelines and will request a string review from l10n
• [] If needed, I updated documentation and added comments to complex code

[adudenamedruby]

Merging this change in requires we first get the AS change in first, because we're failing builds otherwise

[mashalifshin]

Merging this change in requires we first get the AS change in first, because we're failing builds otherwise

Thanks for flagging this @adudenamedruby (and nice to see you and collab with you, been a little while! ).

It's my first time doing this dance so want to make sure I get this right, I'm following the steps here, I'll try to make sure the timing work out so that I get the Android and iOS changes for the version bump in before the next day's build.

[adudenamedruby]

@mergify rebase

[mergify]

rebase

🛑 The pull request rule doesn't match anymore

Details

This action has been cancelled.

[mashalifshin]

@adudenamedruby I believe the application-services changes are all ready to go, I'm planning to merge tomorrow so I need to make sure this PR is also good to go.

Do you need to kick off those maintainer-owned workflows? Will you be the one to merge this as a codeowner, or will that power appear for me after those workflows run?

[adudenamedruby]

I will have to merge that. @issammani any guidance here? as far as I know, we need to merge the AS stuff, then rebase this, to get it to run, and then we merge? or do we straight merge?

[mashalifshin]

I will have to merge that. @issammani any guidance here? as far as I know, we need to merge the AS stuff, then rebase this, to get it to run, and then we merge? or do we straight merge?

I'm also working on figuring out the exact steps here, I think since it's an application-services breaking change (instructions here), it goes like

1. @mashalifshin Merge the a-s PR
2. @mashalifshin Trigger a Nightly build
3. @mashalifshin Once it completes, trigger the iOS build
4. @mashalifshin Update this PR to reference the new Nightly build (example in linked instructions)
5. @adudenamedruby ,or another codeowner Merge this PR

[issammani]

I will have to merge that. @issammani any guidance here? as far as I know, we need to merge the AS stuff, then rebase this, to get it to run, and then we merge? or do we straight merge?

I'm also working on figuring out the exact steps here, I think since it's an application-services breaking change (instructions here), it goes like

1. @mashalifshin Merge the a-s PR

2. @mashalifshin Trigger a Nightly build

3. @mashalifshin Once it completes, trigger the iOS build

4. @mashalifshin  Update this PR to reference the new Nightly build (example in linked instructions)

5. @adudenamedruby ,or another codeowner  Merge this PR

Yes. Since this is a breaking change the AS bump PR will fail CI anyways. @mashalifshin nightlies are built every day early morning and the iOS PR ( today's ) is open ~8am UTC.

What I would usually is just cherry pick the commit from the automated PR in this one. This is an example of a PR with a breaking change and this is an example of a commit I cherry picked from this nightly bump PR.

Happy to help get this landed once we merge the AS one. I am keeping tabs on both PRs just in case 😄

[mashalifshin]

I will have to merge that. @issammani any guidance here? as far as I know, we need to merge the AS stuff, then rebase this, to get it to run, and then we merge? or do we straight merge?

I'm also working on figuring out the exact steps here, I think since it's an application-services breaking change (instructions here), it goes like

1. @mashalifshin Merge the a-s PR

2. @mashalifshin Trigger a Nightly build

3. @mashalifshin Once it completes, trigger the iOS build

4. @mashalifshin  Update this PR to reference the new Nightly build (example in linked instructions)

5. @adudenamedruby ,or another codeowner  Merge this PR

Yes. Since this is a breaking change the AS bump PR will fail CI anyways. @mashalifshin nightlies are built every day early morning and the iOS PR ( today's ) is open ~8am UTC.

What I would usually is just cherry pick the commit from the automated PR in this one. This is an example of a PR with a breaking change and this is an example of a commit I cherry picked from this nightly bump PR.

Happy to help get this landed once we merge the AS one. I am keeping tabs on both PRs just in case 😄

Thank you so much @issammani , the examples are particularly helpful and I appreciate the help getting this landed! Phab patches are now out to the Desktop and Android reviewers, so once those are ready to go we'll be set up to merge. I'll post another comment here before I merge the app-services change