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

[ON HOLD][Tracking] [External] Attendee Tracking project #44725

Open
Julesssss opened this issue Jul 2, 2024 · 48 comments
Open

[ON HOLD][Tracking] [External] Attendee Tracking project #44725

Julesssss opened this issue Jul 2, 2024 · 48 comments
Assignees
Labels
Monthly KSv2 NewFeature Something to build that is a new item.

Comments

@Julesssss
Copy link
Contributor

Julesssss commented Jul 2, 2024

Important Links

Screenshot 2024-07-18 at 14 49 04

Implementation Issues

Frontend:

Backend

TODO

Finish the project

Post-launch polish:

  • Clear out uses of the now-unused Auth attendee param
  • Prevent violation bug by returning OnyxData like done here for other updates
  • Add more tests like testRequestMoneyPersistsAttendees for edit case
  • Attendee tracking feature should be a sub-category of 'Rules' #50802
  • Once fully migrated, introduce minimum attendee size of 1:
    Request::verifyAttributeSize(request, "attendees", 0, Request::MAX_SIZE_NONCOLUMN);
@Julesssss Julesssss added Weekly KSv2 NewFeature Something to build that is a new item. labels Jul 2, 2024
@Expensify Expensify deleted a comment from melvin-bot bot Jul 2, 2024
@Expensify Expensify deleted a comment from melvin-bot bot Jul 2, 2024
@Expensify Expensify deleted a comment from melvin-bot bot Jul 2, 2024
@Julesssss
Copy link
Contributor Author

After discussion here, I want to make the document changes much clearer before seeking help from SWM.

@trjExpensify
Copy link
Contributor

I think this is going in #wave-control, right @JmillsExpensify?

@zsgreenwald
Copy link
Contributor

Back from OOO, @Julesssss am I needed anywhere in particular to keep this moving?

@melvin-bot melvin-bot bot removed the Overdue label Jul 16, 2024
@Julesssss
Copy link
Contributor Author

Hey, it would be helpful to resolve this comment threads, but other than that no, I'll try to hand over the doc to SWM this week.

@Julesssss
Copy link
Contributor Author

@zsgreenwald actually if you had a chance to check the high-level document again today that would be helpful. I tried to update any section that mentioned the old UI (Avatar instead of Name, etc), but there's a chance I might have missed something.

Then we should be able to open the project up to SWM tomorrow 🤞

@Julesssss
Copy link
Contributor Author

The design document for this project is now ready for the external teams to make a start on.

myself and @zsgreenwald will be available for any concerns or questions you have while we work on this project. I have added document comments in places that I think are outdated or need changes. Please feel free to resolve these as you work through the document. And to create new comments with any questions you have for us!

Plan should be for the SWM team:

  • Read the high-level section for background on the project
  • Review, update and improve the detailed section of the document. Paying attention to:
    • Design changes, mainly the switch from Attendee Pills to a Text list of attendees (screenshots below)
    • Codebase changes and refactors since November which make some document details irrelevant/outdated
  • Submit the proposed detailed changes for another round of internal feedback
  • Start work on the feature, while I handle the back-end changes

Previous Design

Screenshot 2024-06-28 at 14 32 00

Updated Design

Screenshot 2024-07-18 at 14 49 04

@filip-solecki
Copy link
Contributor

Hi! I am Filip from SWM and I'd like to work on this issue!

@zfurtak
Copy link
Contributor

zfurtak commented Jul 19, 2024

Hello, I'm Zuza from SWM and I'd like to work on this issue with Filip :)

@Julesssss
Copy link
Contributor Author

@dylanexpensify just holding on this briefly -- we need to switch focus to a SmartScan issue that arose from our backend changes to EditRequest

@Julesssss
Copy link
Contributor Author

@Julesssss
Copy link
Contributor Author

Small update, but I am working on the blocking backend issue as my top priority.

@melvin-bot melvin-bot bot added the Overdue label Oct 1, 2024
@Julesssss
Copy link
Contributor Author

Julesssss commented Oct 2, 2024

Attendee Tracking is currently my top priority, I am making progress with the backend PR that unlocks the other two remaining issues.

I had to switch focus to HybridApp release to resolve a fire, but this is now complete

@melvin-bot melvin-bot bot removed the Overdue label Oct 2, 2024
@melvin-bot melvin-bot bot added the Overdue label Oct 10, 2024
@Julesssss
Copy link
Contributor Author

Update:

I have been making slow but steady progress with the new Auth command, while testing the full flow via Web to client. It's likely I will be submitting the Auth command PR soon which passes back attendee data and creates the new action -- but will not update the recent attendees NVP. I'll do that later.

When the Auth PR is merged, Web-E will be next, then I can open up the App issue so that we can adjust the API param names and onyx keys.

@Julesssss
Copy link
Contributor Author

Internal prioritisation means that this project needs to wind down.

Thankfully the past few weeks has put the project in a good place; the new command is mostly implemented and the onyx attendee data is being passed back to the App clients. I will get the PRs merged, and leave detailed notes on what remains to be done here.

Finish the project

  • Within createUpdateMoneyRequestAttendees, update the users recent attendees and pass back to client
  • App client should update the param used to pass attendees to backend
  • App client should switch to the transaction.attendees onyx keys

Post-launch polish:

@zanyrenney
Copy link
Contributor

When this project is picked back up, mind letting me know? I have a lot of other priorities so I have the help site document ready but I don't want to sink more time into this if it's on pause for now @Julesssss 🙏🏼 Thanks!

@Julesssss
Copy link
Contributor Author

Yeah, so that I don't forget I assigned myself to your issue and will check in monthly.

@melvin-bot melvin-bot bot added Reviewing Has a PR in review Weekly KSv2 and removed Weekly KSv2 labels Oct 23, 2024
@Julesssss
Copy link
Contributor Author

The task list has been updated. The App feature branch is ready to be merged today and is currently in review.

@Julesssss
Copy link
Contributor Author

Julesssss commented Oct 25, 2024

Update regarding the project wind down:

  • App feature branch was merged -- this is huge as it contains all our client changes, and I no longer have to keep resolving conflicts with main 🎉
  • Web-E branch is blocked, it is not worth making more auth changes in order to finish up a single automated test case. However I think we can easily leave this change until the project is picked back up, so I have officially marked the 'create new command' task as done
  • Auth changes were merged already, including automated tests 🎉

@ZhenjaHorbach
Copy link
Contributor

ZhenjaHorbach commented Oct 25, 2024

And one more thing 😅
Since the project will be wind down indefinitely
Could I please get paymeny so as not to forget later ? ( I reviewed 3 PR (4 issues))

@Julesssss
Copy link
Contributor Author

Of course 😄

Please correct me if I forgot, but I think these are the PRs you reviewed?

@ZhenjaHorbach
Copy link
Contributor

Of course 😄

Please correct me if I forgot, but I think these are the PRs you reviewed?

Yeap
Everything is correct !

@Julesssss Julesssss changed the title [Tracking] [External] Attendee Tracking project [ON HOLD][Tracking] [External] Attendee Tracking project Oct 25, 2024
@garrettmknight garrettmknight moved this from Tracking to Future #migrate Project in [#whatsnext] #expense Oct 30, 2024
@garrettmknight garrettmknight added Monthly KSv2 and removed Reviewing Has a PR in review Weekly KSv2 labels Oct 30, 2024
@JmillsExpensify
Copy link

Still on hold

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Monthly KSv2 NewFeature Something to build that is a new item.
Projects
Status: Future Cohort Project
Development

No branches or pull requests