text:
Refactor Events to use Visitor Pattern

Visitor Pattern decouples logic from input element. In practice this means we can
get rid of the ugly instanceof checks in the TournamentServiceController.

Codecov Report

Merging #25 into develop will increase coverage by 0.14%.
The diff coverage is 90.9%.

@@             Coverage Diff              @@
##             develop     #25      +/-   ##
============================================
+ Coverage      47.35%   47.5%   +0.14%
- Complexity       327     329       +2
============================================
  Files             89      89
  Lines           1438    1440       +2
  Branches          90      89       -1
============================================
+ Hits             681     684       +3
  Misses           717     717
+ Partials          40      39       -1

Impacted Files
Coverage Δ
Complexity Δ

...son/smashapp/common/domain/event/EventFactory.java
100% <ø> (ø)
3 <0> (ø)
:arrow_down:

...app/tournament/presentation/TournamentService.java
0% <0%> (ø)
0 <0> (ø)
:arrow_down:

...ment/presentation/TournamentServiceController.java
100% <100%> (+9.09%)
3 <2> (ø)
:arrow_down:

...ournament/domain/event/SearchTournamentAction.java
100% <100%> (ø)
2 <2> (?)

...ikkarlsson/smashapp/common/domain/event/Event.java
100% <100%> (ø)
1 <1> (ø)
:arrow_down:

...tournament/domain/event/FetchTournamentAction.java
100% <100%> (ø)
3 <1> (?)

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update cc2c77c...0680b1a. Read the comment docs.

authors: ErikKarlsson codecov-io

repo: ErikKarlsson/SmashApp-Android


text:
Missing requirement Cryptodome?

When installing this package from pypi (pip install bleparser) the Cryptodome package does not get installed, which seems to be a hard requirement (https://github.com/Ernst79/bleparser/blob/master/package/bleparser/xiaomi.py#L5).
Should be added into setup.cfg under "install_requires".
I would be happy to draft a PR for this, but a little unsure about the requirements_test.txt in this repo which specifies a specific version of Cryptodome. Any particular reason for pinning the version or would any version >3 be ok?

I was under the impression that packages = find: would take care of that, but apparently not.
Any version of Cryptodome is ok, using the latest would be preferred, I think.

source: gharchive/issue

license: MIT

license_type: permissive

license_source: github-api


text:
Request for Mac Support

As a Mac user, I kindly request you to consider extending your support to the MacOS platform.

Sorry, I've never used a Mac, and I don't know anything about MacOS.

repo: ErrorFlynn/ytdlp-interface


text:
Analysis tools fail when field visibility modified on input layer

repro steps:
change field visibility on attribute table
pass layer into analysis tools

get validation error

verified

repo: Esri/environmental-impact-tools


text:
Date field also input with the time, how could to get rid of the time but just keep the date

There is date field for my data, every time I input the date it automatically come with the time as well. May I get rid of the time but just keep the date.

I'll add an option for this. Currently the time shows at the bottom but that can be disabled.

Could you let me know how to disable the time? I only like to show the dat.
From: Matt Driscoll [mailto:notifications@github.com]
Sent: Monday, October 24, 2016 1:41 PM
To: Esri/geoform-template-js geoform-template-js@noreply.github.com
Cc: Xia, Zhujing ZhujingX@co.rockland.ny.us; Author author@noreply.github.com
Subject: Re: [Esri/geoform-template-js] Date field also input with the time, how could to get rid of the time but just keep the date (#520)
ATTENTION: This email came from an external source. Do not open attachments or click on links from unknown senders or unexpected emails.
I'll add an option for this. Currently the time shows at the bottom but that can be disabled.
—
You are receiving this because you authored the thread.
Reply to this email directly, view it on GitHubhttps://github.com/Esri/geoform-template-js/issues/520#issuecomment-255811412, or mute the threadhttps://github.com/notifications/unsubscribe-auth/ASx7I8VOiTjZN1z0fU1OkSOQ2DdWTl86ks5q3O2cgaJpZM4KbTEW.

Found the main.js line 86, and change the format to ‘L’. Thanks
From: Matt Driscoll [mailto:notifications@github.com]
Sent: Monday, October 24, 2016 1:41 PM
To: Esri/geoform-template-js geoform-template-js@noreply.github.com
Cc: Xia, Zhujing ZhujingX@co.rockland.ny.us; Author author@noreply.github.com
Subject: Re: [Esri/geoform-template-js] Date field also input with the time, how could to get rid of the time but just keep the date (#520)
ATTENTION: This email came from an external source. Do not open attachments or click on links from unknown senders or unexpected emails.
I'll add an option for this. Currently the time shows at the bottom but that can be disabled.
—
You are receiving this because you authored the thread.
Reply to this email directly, view it on GitHubhttps://github.com/Esri/geoform-template-js/issues/520#issuecomment-255811412, or mute the threadhttps://github.com/notifications/unsubscribe-auth/ASx7I8VOiTjZN1z0fU1OkSOQ2DdWTl86ks5q3O2cgaJpZM4KbTEW.

We also just ran across this, was it added in a recent release or still pending? @driskull

Its still pending @slibby. Thanks

authors: driskull slibby zhujingx

repo: Esri/geoform-template-js

license_source: bigquery


text:
Update to JSAPI 3.11

iOS 8 Safari Issue zoom in and out buttons do not work. This issue exists on iphone and ipad iOS 8.0.2

in MASTER branch for FEB

repo: Esri/live-work-locate


text:
Coordinate Conversion does not load

This is the initial configuration when you select the widget.

@kgonzago was able to find the issue. When using a widget in a hosted portal you must make sure that all files are named properly. The settings files and feeback.json was lower case and needed to be uppercase. This is not an issue when using the developer edition, but is on a hosted instance of portal.

repo: Esri/military-tools-webappbuilder


text:
[错误报告] 日志显示重命名失败，实际成功了

当前程序版本
2.6.0
问题类型
其他问题
问题描述
日志显示重命名失败，实际成功了
PS：顺便通知功能不工作，不知是否和上述问题有联系
发生问题时系统日志
[2023-04-27 00:44:43,245] INFO:     [Lilith-Raws] Kaminaki Sekai no Kamisama Katsudou - 04 [Baha][WEB-DL][1080p][AVC AAC][CHT][MP4].mp4 >> Kaminaki Sekai no Kamisama Katsudou S01E04.mp4
[2023-04-27 00:44:43,245] WARNING:  [Lilith-Raws] Kaminaki Sekai no Kamisama Katsudou - 04 [Baha][WEB-DL][1080p][AVC AAC][CHT][MP4].mp4 rename failed
[2023-04-27 00:44:43,246] WARNING:  Folder name: Kaminaki Sekai no Kamisama Katsudou, Season: 1, Suffix: .mp4
[2023-04-27 00:44:43,278] INFO:     [Lilith-Raws] Kono Subarashii Sekai ni Bakuen wo! - 04 [Baha][WEB-DL][1080p][AVC AAC][CHT][MP4].mp4 >> Kono Subarashii Sekai ni Bakuen wo! S01E04.mp4
[2023-04-27 00:44:43,279] WARNING:  [Lilith-Raws] Kono Subarashii Sekai ni Bakuen wo! - 04 [Baha][WEB-DL][1080p][AVC AAC][CHT][MP4].mp4 rename failed
[2023-04-27 00:44:43,279] WARNING:  Folder name: Kono Subarashii Sekai ni Bakuen wo!, Season: 1, Suffix: .mp4

通知用的什么形式

已定位到 bug 下个版本修复

通知用的什么形式

Telegram

authors: EstrellaXD sakuyamaij


text:
Added new functional

Possibility adding state of the request. By default state is latest. User can set other state for example pending, latest...

Added new function "send_raw_command". This function very useful when user want send the raw command using this client.

hi  @artamonovdev
Thank you for PR!
I added proposition for improvements in the code. Would be great if you can apply them and I'll be happy to merge.

Why you don't want merge this pull-request?

@artamonovdev, I was hoping you will implement changes proposed in comments first.

@marekkirejczyk I what to know the pending balance of a wallet. But with this library I can find out only "latest" balance of a wallet.

source: gharchive/pull-request

authors: artamonovdev marekkirejczyk

license: mit


text:
[ethicapp-v2] Issue #200, #214: Explicación sobre tipos de diseños y arreglos visuales en la interfaz del profesor

En este PR se implementa una solución para proveer mayor explicación de los tipos de diseños para actividades. Además hay varios arreglos de layout en las secciones de actividad, diseños y de crear un diseño.

El commit que realizé fue para arreglar un par de errores que encontré usando los linters fueron poco y sólo en css así que decidí hacer los cambios yo, pero trata de usar los linters para mantener el orden del código @javiersoto99.

@javiersoto99 ¿Puedes postear pantallazos para ver el resultado por aquí antes de aceptar?

Apruebo las mejoras realizadas!

Entendido! Iré subiendo pantallazos adjuntos en los PRs para facilitar la revisión.

authors: claudio-alvarez javiersoto99 mabarraza

repo: EthicApp-Development/ethicapp-main


text:
Accessibility colors using "prefers-contrast" media query

I noticed that on the Apple website there are differences in the system colors when a specific accessibility setting is changed. I'm not familiar with this. Would something like this be considered?

https://developer.apple.com/design/human-interface-guidelines/color#Specifications

Perhaps this CSS media query could be used as a fallback on web, but bundle size is important on web so maybe not worth the extra code https://developer.mozilla.org/en-US/docs/Web/CSS/@media/prefers-contrast

repo: EvanBacon/expo-apple-colors


text:
Accuracy and F1 score when using ViT-base

Hi. Thank you for sharing your great work.
I wonder how much is the mA and F1 score if SequencePAR and PromptPAR backbone is changed into ViT-b/16 on PA100k, PETA, RAPv2 dataset.
If you conducted the experiments, could you kindly share in public?
Thank you.

Sorry, we have not experimented with replacing the backbone on more datasets, if you are interested, you can try to reproduce the code for your experiments, or we will update these results later!

Thank you :)

repo: Event-AHU/OpenPAR


text:
Fuelux2 intelligent dropdown cyclical dependency fix #1123

3 unit tests forward and 2 back
Unfortunately qunit appears to be preventing bubbling when bs dropdown toggle returns false where as the browser handles it fine.
Integration test handled in the browser.
@kevinparkerson this will need your v2 experience

I don't have time to look at this at the moment. @vernak2539 would you be willing / able to take a peek?

@kevinparkerson The only thing that @swilliamset is asking for is if removing fuelux/all will break anything. Otherwise it's just unit tests, and the rest of us can review the Qunit issues.

repo: ExactTarget/fuelux


text:
Method 1 update suggestion

After playing with this for a few days, I think it would be helpful to include initial capitalised consecutive words in the quote search.
Example:
Which English language idiom is also the title of an episode of Under The Dome?
['Under The Weather', 'Watching Paint Dry', 'Thicker Than Water']

Searching
['Under The Weather', 'Watching Paint Dry', 'Thicker Than Water']
['english', 'language', 'idiom', 'also', 'title', 'episode', 'dome']
['https://en.wikipedia.org/wiki/D%27oh!', 'https://en.wikisource.org/wiki/English_as_we_speak_it_in_Ireland/IV', 'https://theartofbusinessenglish.com/2018/02/22/idioms-for-describing-change-when-presenting/', 'https://player.fm/podcasts/Idioms', 'http://www.bbc.co.uk/worldservice/learningenglish/youmeus/learnit/learnitv167.shtml']
Running method 1
{'under the weather': 0, 'watching paint dry': 4, 'thicker than water': 0}
watching paint dry

[('Which', 'JJ'), ('English', 'NNP'), ('language', 'NN'), ('idiom', 'NN'), ('is', 'VBZ'), ('also', 'RB'), ('the', 'DT'), ('title', 'NN'), ('of', 'IN'), ('an', 'DT'), ('episode', 'NN'), ('of', 'IN'), ('Under', 'IN'), ('The', 'DT'), ('Dome', 'NNP'), ('?', '.')]
Question nouns: ['dome']
Running method 3
Search processed
URLs fetched

Under The Weather: {'dome': 0}
Watching Paint Dry: {'dome': 0}
Thicker Than Water: {'dome': 2}

Keyword scores: {'Under The Weather': 73, 'Watching Paint Dry': 25, 'Thicker Than Water': 42}
Noun scores: {'Under The Weather': 0, 'Watching Paint Dry': 0, 'Thicker Than Water': 2}
Thicker Than Water

Whilst method 3 got the correct answer, just on the off-chance that dome was a key noun word (which doesn't ever seem to work except in rare circumstances), method 1 really failed here.
Googling this question myself (I've been watching some of these for fun btw) does come up with results that would be parseable by the script to give the correct answer if the above suggestion was incorporated.
I'm not too great with Python, but a simple re regex for [A-Z] in here somewhere would implement this pretty painlessly:
    quoted = re.findall('"([^"]*)"', question_lower)  # Get all words in quotes
    no_quote = question_lower
    for quote in quoted:
        no_quote = no_quote.replace(f"\"{quote}\"", "1placeholder1")

    question_keywords = cssearch.find_keywords(no_quote)
    for quote in quoted:
        question_keywords[question_keywords.index("1placeholder1")] = quote

Thanks for the suggestion, added in https://github.com/Exaphis/HackQ-Trivia/commit/5269bb55278297e2dc15bb0365bc02ef72a80a16.

authors: Exaphis talentoscope

repo: Exaphis/HackQ-Trivia


text:
feat: add ens name and avatar support

cc @melondrone @
HOVOH

@HOVOH any ideas when this will get deployed? If available in prod we wll add you to our ens.domains homepage 🙇🏻

repo: ExodiaFinance/exodia-frontend


text:
[BUG] [1.12] Fishing Rod crosshair instability

When in Shoulder Surfing mode and holding a Fishing Rod, by simply moving about the terrain, there is no stability in the crosshair's y-axis. It seems to only affect this item, but as someone who uses it in combat, it can be slightly distracting.
To reproduce, it should become apparent when comparing a Fishing Rod to any other item, that the crosshair bounces up and down and appears lower than it should.
Issue happened in latest 1.12 compatible version of the mod, with only Optifine installed and in a Single Player survival world.
Also, thank you so much for fixing the reach issue, I am very grateful! Love your work. :)

My bad sorry, didn't realise this was a setting. Disabled it and it's perfect for my playstyle. Thank you!

repo: Exopandora/ShoulderSurfing


text:
My first branch

made some change
added profile.md

good to go!


text:
File name with  "/"  removes the extension and breaks the file when downloaded

If you haven’t already, check out our contributing guidelines for onboarding and email contributors@expensify.com to request to join our Slack channel!

Action Performed:

Open Expensify
Open any report
Attach any file with "/" as the filename and valid file extension.
Download the file.

Expected Result:
Valid file downloaded
Actual Result:
Malformed file downloaded due to removed extension
Workaround:
unknown
Platforms:

Which of our officially supported platforms is this issue occurring on?

[ ] Android / native
[ ] Android / Chrome
[ ] iOS / native
[ ] iOS / Safari
[ ] MacOS / Chrome / Safari
[ ] MacOS / Desktop

Version Number: 1.2.53-0
Reproducible in staging?: y
Reproducible in production?: y
If this was caught during regression testing, add the test name, ID and link from TestRail:
Email or phone of affected tester (no customers):
Logs: https://stackoverflow.com/c/expensify/questions/4856
Notes/Photos/Videos:
https://user-images.githubusercontent.com/43996225/212224807-7f129ec1-22aa-4a2c-b9f1-47ef3cd6f466.mov
https://user-images.githubusercontent.com/43996225/212225197-7fa16ef6-626a-4a4d-9f54-80ecb9d50c8c.mp4
Expensify/Expensify Issue URL:
Issue reported by: @oesayan
Slack conversation: https://expensify.slack.com/archives/C049HHMV9SM/p1673509312939899
View all open jobs on GitHub
Upwork Automation - Do Not Edit

Upwork Job URL: https://www.upwork.com/jobs/~01b3bd35954214a174
Upwork Job ID: 1614013175418511360
Last Price Increase: 2023-01-13

In the thread @mountiny mentioned this being internal, @dangrous please let me know otherwise!

@dangrous i think it could have solution in both, but ideally we would make sure this is processed fine in the backend (havnet looked into where is the root cause of this)

bump @dangrous 👋

Hi sorry - was OOO yesterday! Will look into this today.

Okay so figured out part of it -
When a user uploads an attachment, we sanitize the filename. This would turn, for example, /.png into just .png - we use this:
preg_replace('/[^a-zA-Z0-9\-\._]/', '', $filename);
The / actually gets converted into a : at some point in the process, so the regex also filters out that.
However, after that point I've confirmed that the filetype is correct, so I'm not sure why it's downloading as png.txt.
But either way, if we allow colons in the filename, the behavior is resolved. It downloads as _.png but you are able to open it fine. @mountiny what do you think about this? Is there an insecurity in allowing colons, or can you think of other behavior that might get messed up?

@dangrous  I think one reason for not using : colons is Windows doesn't support : in the file names. So when saving the files the users would probably get an error. Are you saying that : gets converted into _ underscore when the file is downloaded? Then that should be just fine.

I cant really confidently answer if this opens door for some other issues.
I would say we could also not allow the / in file name, I dont think its very common to have filenames with this character.
But as Manan mentioned, I think converting it to underscore and running with that should be fine if we want just backend solution

So at some point in the flow (I'm not sure where yet) the / gets converted into a : - this happens on the App side somewhere.
Then, the : is stripped out on the Web-E side.
After changing the regex in testing to allow :, somewhere between storing the file and downloading it, that : gets converted into a _.
So maybe if I can find where /.png becomes :.png on the App side, we could make it switch it into _.png and then we'd be all set. That could also be external, too.

There's a lot of somewheres. So are we saying this could purely be external? with no change on the backend?

I think we should do this in both, change the / to _ in frontend and backend as well so we not only rely on the frontend and make sure we dont leave room for vulnerabilities.

That's fair. So I can put up a PR that adjusts https://github.com/Expensify/Web-Expensify/blob/62f13038a4fe5e3f9991f1d2b23e7a00474a1827/lib/ReportUtils.php#L2221-L2224 to also replace any /s and :s with _s, and then we can open this externally to do the same on the App side?
For reference, I know it's getting switched to a : on the app side because when you attempt to upload a /.png the header says:

Before you hit the server at all.

I think the / to : separator is done by the MacOS (*nix).

and then when we get the file from the backend, we don't get any / or :.  The originalMessage content received from the backend.
<img src="https://www.expensify.com/chat-attachments/4313048699244575240/w_3249cb0371a0cac30c660b74ee79935ee9d0c9b0.png.1024.jpg" data-expensify-source="https://www.expensify.com/chat-attachments/4313048699244575240/w_3249cb0371a0cac30c660b74ee79935ee9d0c9b0.png" data-name=".png" data-expensify-width="225" data-expensify-height="224" />

Notice the data-name which only contains .png and hence we save the file it shows just png.

Ah okay. So we'd need to add something there to switch it to a _ but that should be fine. The data-name is only showing png because it goes through that regex above.
When I replace the regex on the backend to swap out : and /, it succesfully downloads from the front end as [an underscore for each slash].png so looks like it's working there, even without a frontend change.

@mananjadhav is right - that's an old tale in MacOS world since before some old version MacOS used : instead of slashes and some layers of MacOS still use :, some layers use slash - Finder allows slashes to be used in filenames, while some other layers do not use slashes and replace them with : .
That's why this behaviour is the same on iOS and MacOS, however on Android the OS replaces the slashes with _.

I think @akshayasalvi's proposal is good here. One recommendation that, move the .replace call to FileUtils.

but if it needs to be done for the native too, I can do that.

Yes, let's add the check for native too.
@dangrous @mountiny What do you folks think?

I think thats a good step and I think we should also explore updating this in backend to match the client behaviour.
Did we test this will fix the download issue?

@mountiny So I tried playing around with it a little bit more and noticed the same behaviour if the file is named with emojis.

@mountiny I tested the code and it works fine with the download.
But there's a revised proposal here. Because we can't change the name of a File object, we need to create another instance. So the lines change as:
file.uri = URL.createObjectURL(file);
const updatedFile = new File([file], file.name.replace(/[/:]/g, '_').replace(/_+/g, '_')); // new file with the new name
this.onPicked(updatedFile); // send the file with the new name

Thanks for the revised proposal @akshayasalvi. I was just testing your revised changes and it seems to be working.
https://user-images.githubusercontent.com/3069065/213466152-6ec1f5f7-0f40-41a4-944c-27a1cfe2c58a.mov
cc - @mountiny

Thanks for the discussion, I will let @dangrous handle it since he is assigned 😄

I think this makes sense - but I know little about file pickers. This wouldn't end up taking double the space or anything, right? Either for us or for the user (are we creating a duplicate file on their device)?
Separately, two thoughts about the regex:

What's the reasoning behind consolidating multiple /s and : into only one _? It's probably fine, but doesn't seem necessary.
If we do want that, I think we can just do replace(/[/:]+/g, '_') vs. replace(/[/:]/g, '_').replace(/_+/g, '_') and get the same result

Let me know what you all think, and once we're confirmed on the direction (single underscore vs. multiple) I can make the PR for the backend, as well.

@dangrous @mananjadhav
Hey guys, I guess you missed my comment before.
I am the guy who found the issue and I played with it around a little more and found the exact same behavior with emojis, arabic symbols and probably some other symbols too.
So you will probably need a regex not only changing the / and :, but everything except alphanumeric characters.

@dangrous You're right we can update the regex.

The reason for multiple / or :, is that if we have a file: :::.png or /:.png, it would get renamed as ___.png and __.png.

It doesn't create a duplicate file on the client side, just a temp file, because you can't update the name of uploaded file.

re @oesayan - thanks for reposting, just confirmed! so maybe we want replace(/[^a-zA-Z0-9\-\._]+/g, '_') which would be everything not letters, numbers, -, ., and _. I think just for personal preference, let's do one _ per symbol, so not group them. (I'll make the same adjustments for the backend PR.)
@mountiny you can't think of any reason why that would cause an issue, can you?

Okay I can work without grouping too. Then I just need to remove the second regex. I can also update the regex to the one you gave replace(/[^a-zA-Z0-9\-\._]+/g, '_') to just keep letters, numbers, and limited special characters.
Let me know @dangrous if any other changes are expected here

@dangrous Nope, I think this should not break anything for sure.
Making this External as well since contributor will work on this issue too

@mountiny Is this issue looking for any new proposals or the above proposal has already been selected?

Good question, thanks @priyeshshah11, I think we might be good here and apologies this was not made External earlier for fairer conditions. We havent been sure what is the best way forward.
@dangrous if you want more proposals though, feel free to add the Help Wanted label back

Agh, sorry for the process confusion! I missed that this wasn't already External, since it was unlocked. I'll definitely make sure to check that everything's in the right state for future issues to make everything fairer!
I think for now though, the proposal put forward by @akshayasalvi makes sense, with the edits noted here, I will assign you and we can move ahead with the PR.
I'll spin up the backend update as well; we don't need to hold on either one, but it'll be good to get them out relatively close to each other.

The backend fix is deployed to staging! Soon to be on prod. @akshayasalvi how are we looking on the PR for the front end part of this?

@akshayasalvi How are things going with the PR? We'd love to get that merged this week, and it seems like a pretty quick fix!

@dangrous It will be up in an hour.

Currently testing this one.

Still waiting on this to get pushed to prod but should be soon!

@dangrous @mananjadhav @akshayasalvi
Associated PR seems to raise a regression where we won't able to reach out to Avatar Crop part and the infinite loader will remain on the screen after cleaning the filename.
Reason is uri will be undefined as the file object is overwritten here.
https://github.com/Expensify/App/blob/51f1a77e18f222806da32c5d6670d2de5267fa6f/src/components/AttachmentPicker/index.js#L39-L42
https://user-images.githubusercontent.com/14358475/216611655-e81806b3-9cb6-4670-b288-8aced226e179.mp4
To solve we need to make an order change
diff --git a/src/components/AttachmentPicker/index.js b/src/components/AttachmentPicker/index.js
index dddceb7cf7..dc6573e02c 100644
--- a/src/components/AttachmentPicker/index.js
+++ b/src/components/AttachmentPicker/index.js
@@ -36,10 +36,10 @@ class AttachmentPicker extends React.Component {

                         if (file) {
                             const cleanName = FileUtils.cleanFileName(file.name);
-                            file.uri = URL.createObjectURL(file);
                             if (file.name !== cleanName) {
                                 file = new File([file], cleanName);
                             }
+                            file.uri = URL.createObjectURL(file);
                             this.onPicked(file);

https://user-images.githubusercontent.com/14358475/216611547-b1dc061c-913d-4533-917d-17e4f3348759.mov
Context for me - https://github.com/Expensify/App/issues/14757

Re regression - yes, it looks like this did cause a regression, based on @Pujan92's analysis. I also tried reverting the change locally and confirmed that behavior was correct prior to the change.
I'm not sure if a checklist change is needed; I think the thing that was missed here was honestly just proofreading - I should have noticed that the file would no longer have the uri if the name was changed. I guess technically we could have noticed the bug by testing all places where attachments were uploaded, rather than just as attachments - but I'm not sure that's the primary solution here.

@dangrous I just went through the comments. I didn't think upload Avatar will be any different as it is the same picker.  I can see another PR is merged, is there any action pending here for me?

Hey @akshayasalvi so to clarify (this is confusing!) the step on the checklist above is actually to propose a regression test for the original bug - that is, that /.png etc. won't be able to be downloaded. I think it would be something along the lines of "Upload a file named ///.png and make sure you can download it." but formatted appropriately according to the link.
Does that make sense?

Separately, @mananjadhav, I think this is likely just an edge case we didn't consider before, rather than a regression. The initial renaming looks like it was added 7/24/20 into the backend so it was quite some time ago (possibly before the App existed?). Therefore no PR to comment on, and I don't think we need the discussion, as long as we get the new regression test. Does that sound right to you?

Bump @akshayasalvi and @mananjadhav on the above - thanks!

Regression Test Proposal
Bug: File name with special characters like "/" breaks the file download
Proposed Test Steps:

Go to URL https://staging.new.expensify.com/
Log in with any account & navigate to any conversation
Have an image/file with the name as : or / with the extension, for example, ///.png.
In the selected conversation, upload the above file
Verify that the preview should show if it is an image and all occurrences of the special character '/' is replaced with _
Click on upload and ensure the file is successfully uploaded
Open the file in the attachment preview once uploaded
Ensure you're able to preview if it is an image
Click on download for the image/file and once the file is downloaded, it should open successfully.

@dangrous @mananjadhav  Is this okay for the test proposal?

I was OOO and just getting back, waiting for @akshayasalvi to post the regression test.

Hi @akshayasalvi this looks great! A couple edits - primarily I think we should just be very specific (you're right that it doesn't actually matter the kind of file, etc. but it'll be easier to test if it's particular).

Go to URL staging.new.expensify.com.
Log in with any account & navigate to any conversation.
Have an image with the name ///.png.
Upload the file to the conversation as an attachment.
Verify that the upload preview shows the image correctly, and the name of the file is shown as ___.png (all occurrences of the special character / are replaced with _).
Click on upload and ensure the file is successfully uploaded.
Open the file in the attachment preview.
Ensure you're able to preview the image.
Download the image and once the file is downloaded (as ___.png), ensure it opens successfully.

How does that sound?

@mananjadhav @akshayasalvi @oesayan - can you three please apply to the job so I can issue payment? https://www.upwork.com/jobs/~01b3bd35954214a174

@NicMendonca applied, thank you.
@dangrous Yes this sounds good. It's my first time writing the regression test proposal. thank you for the help.

@NicMendonca I've applied too.

@NicMendonca and I have applied too 🤠

@mananjadhav @akshayasalvi  - paid, thank you!
@oesayan - I'll issue payment once you accept the offer, thanks!

everyone had been paid! thank you!!

authors: NicMendonca Pujan92 akshayasalvi dangrous kavimuru mananjadhav mountiny oesayan priyeshshah11

repo: Expensify/App


text:
Chat - Error message appears when uploaded the PDF file offline

If you haven’t already, check out our contributing guidelines for onboarding and email contributors@expensify.com to request to join our Slack channel!

Action Performed:

Launch App or go to staging.new.expensify.com/
Login with any account
Go to any chat
Send PDF file to the conversation (so it's not cached)
Disable the internet connection in the device
Wait a few seconds and go back online

Expected Result:
After returning to online, the attachment must be display
Actual Result:
Error message appears when uploaded the PDF file offline
Workaround:
Unknown
Platforms:

Which of our officially supported platforms is this issue occurring on?

[ ] Android / native
[ ] Android / Chrome
[ ] iOS / native
[ ] iOS / Safari
[ ] MacOS / Chrome / Safari
[ ] MacOS / Desktop

Version Number: 1.2.65.0
Reproducible in staging?: Yes
Reproducible in production?: No
If this was caught during regression testing, add the test name, ID and link from TestRail:
Email or phone of affected tester (no customers):
Logs: https://stackoverflow.com/c/expensify/questions/4856
Notes/Photos/Videos: Any additional supporting documentation
https://user-images.githubusercontent.com/93399543/216784324-a611ef01-fd45-4c8e-875b-ff1ca5558f65.mp4

Expensify/Expensify Issue URL:
Issue reported by: Applause - Internal Team
Slack conversation:
View all open jobs on GitHub

:wave: Friendly reminder that deploy blockers are time-sensitive ⏱ issues! Check out the open StagingDeployCash deploy checklist to see the list of PRs included in this release, then work quickly to do one of the following:

Identify the pull request that introduced this issue and revert it.
Find someone who can quickly fix the issue.
Fix the issue yourself.

So I am actually wondering if the upload has been working in production, since we added this change https://github.com/Expensify/App/pull/14736 to introduced this generic upload failure message (hence why it cannot be reproduced in production). So maybe this fails in production as well but we just did not handle the failure correctly so this would be error of its own. cc @iwiznia

The error message should not be shown unless the request failed, so I don't think that's the issue?

I was not able to reproduce, asking for help here

Can't reproduce either

In the video we see that they upload first and then disable the connection while the upload has not finished yet. This is slightly different than the step described because the request was sent, maybe that is why you couldn't reproduce.
I don't think we are supposed to handle this case so I agree with closing.

authors: OSBotify aldo-expensify iwiznia kbecciv mountiny


text:
[HOLD for payment 2023-03-16] Locale Picker Background is incorrect on windows (DEV)

The Locale Picker Background should match app background on windows. This is occurring on DEV only awaiting staging release.
https://expensify.slack.com/archives/C049HHMV9SM/p1678032527107149
Reported by @situchan

This is ONLY on windows web.
Not MacOS/Android/IOS
Upwork Automation - Do Not Edit

Upwork Job URL: https://www.upwork.com/jobs/~01806153de5b609fd1
Upwork Job ID: 1632693916961329152
Last Price Increase: 2023-03-06

Job added to Upwork: https://www.upwork.com/jobs/~01806153de5b609fd1

Triggered auto assignment to @joekaufmanexpensify (Bug), see https://stackoverflow.com/c/expensify/questions/14418 for more details.

Bug0 Triage Checklist (Main S/O)

[ ] This "bug" occurs on a supported platform (ensure Platforms in OP are ✅)
[ ] This bug is not a duplicate report (check E/App issues and #expensify-bugs)

If it is, comment with a link to the original report, close the issue and add any novel details to the original issue instead

[ ] This bug is reproducible using the reproduction steps in the OP. S/O

If the reproduction steps are clear and you're unable to reproduce the bug, check with the reporter and QA first, then close the issue.
If the reproduction steps aren't clear and you determine the correct steps, please update the OP.

[ ] This issue is filled out as thoroughly and clearly as possible

Pay special attention to the title, results, platforms where the bug occurs, and if the bug happens on staging/production.

[ ] I have reviewed and subscribed to the linked Slack conversation to ensure Slack/Github stay in sync

Triggered auto assignment to Contributor Plus for review of internal employee PR - @sobitneupane (Internal)

In this issue, I also suggest to remove border radius in order to fix https://github.com/Expensify/App/issues/15653, which was added very long time ago.
https://github.com/Expensify/App/blob/228a29b90cbc112743a72c831cab16aceef6e785/src/styles/styles.js#L571
https://github.com/Expensify/App/blob/228a29b90cbc112743a72c831cab16aceef6e785/src/styles/styles.js#L596
https://github.com/Expensify/App/blob/228a29b90cbc112743a72c831cab16aceef6e785/src/styles/styles.js#L612

Looks like the PR for this one is already merged. I'll stay assigned for now as we'll need to pay out the reporting bonus and C+ review payment after the PR is deployed.

@joekaufmanexpensify could you handle payment tracking for this issue as well?
https://github.com/Expensify/App/issues/15653
@s77rt did one C+ review for the PR where we fixed both bugs

Sure! Just to make sure my understanding is correct though, this one PR was used to fix two different bugs, right? If so, I think it'd just be one payment for C+ reviewing the one PR.

@grgia am I also eligible for compensation for providing solutions on both GHs and they were used in PR?

Reviewing label has been removed, please complete the "BugZero Checklist".

The solution for this issue has been :rocket: deployed to production :rocket: in version 1.2.80-2 and is now subject to a 7-day regression period :calendar:. Here is the list of pull requests that resolve this issue:

https://github.com/Expensify/App/pull/15675

If no regressions arise, payment will be issued on 2023-03-16. :confetti_ball:
After the hold period is over and BZ checklist items are completed, please complete any of the applicable payments for this issue, and check them off once done.

[ ] External issue reporter
[ ] Contributor that fixed the issue
[ ] Contributor+ that helped on the issue and/or PR

As a reminder, here are the bonuses/penalties that should be applied for any External issue:

Merged PR within 3 business days of assignment - 50% bonus
Merged PR more than 9 business days after assignment - 50% penalty

BugZero Checklist: The PR fixing this issue has been merged! The following checklist (instructions) will need to be completed before the issue can be closed:

[ ] [@grgia] The PR that introduced the bug has been identified. Link to the PR:
[ ] [@grgia] The offending PR has been commented on, pointing out the bug it caused and why, so the author and reviewers can learn from the mistake. Link to comment:
[ ] [@grgia] A discussion in #expensify-bugs has been started about whether any other steps should be taken (e.g. updating the PR review checklist) in order to catch this type of bug sooner. Link to discussion:
[ ] [@joekaufmanexpensify] Determine if we should create a regression test for the bug.
[ ] [@grgia] If we decide to create a regression test for the bug, please propose the regression test steps to the appropriate location to ensure the same bug will not reach production again.
[ ] [@joekaufmanexpensify] Review the proposed regression test steps and location.
[ ] [@joekaufmanexpensify] Link the GH issue for creating/updating the regression test once above steps have been agreed upon:

Looks like something related to react-navigation may have been mentioned in this issue discussion.
As a reminder, please make sure that all proposals are not workarounds and that any and all attempt to fix the issue holistically have been made before proceeding with a solution. Proposals to change our DeprecatedCustomActions.js files should not be accepted.
Feel free to drop a note in #expensify-open-source with any questions.

The PR that introduced the bug has been identified: https://github.com/Expensify/App/pull/15022
The offending PR has been commented on: https://github.com/Expensify/App/pull/15022#issuecomment-1462131504
A discussion in #expensify-bugs has been started: cc @grgia

@joekaufmanexpensify Please tag me if a regression test is required.

IMO no regression test is needed here. This is a niche bug, and windows only.

Almost ready to issue payment here. 7 day period is up tomorrow. @grgia mind handling the BZ steps related to the PR that introduced the bug when you have a chance?

We need to issue the following payments:

@s77rt - $1,000 for C+ review.
@situchan - $250 for reporting the bug.

@s77rt offer sent for $1000!

@grgia am I also eligible for compensation for providing solutions on both GHs and they were used in PR?

@grgia any chance to review this?

Actually @s77rt, I see @tjferriss already hired you for this PR review in https://github.com/Expensify/App/issues/15653#issuecomment-1467305240 , as the PR in that issue also fixed the bug here. So went ahead and withdrew the offer for this issue.

@situchan , generally we only provide compensation for proposals when the issue in question is marked as external. For those issues, a proposal will be formally reviewed, and then you'll be hired for a job. LMK if you have any other questions!

I have no questions according to contributing guideline. But there might be edge cases. Related discussion is already happening here

We definitely appreciate your help on the issue! I reviewed the Slack thread you linked, and I agree with Ionatan's sentiment here. We may have gone outside of our normal process to provide compensation for assistance on an internal issue in the past, but that isn't part of the standard process, which we generally try to adhere to.

We discussed this internally, and we're going to stick with $250 for this one. LMK once you've accepted the job, and I'll work on issuing payment!

Thanks for confirmation. I am following team's decision as always.
Hope that rule would be applied to all similar cases for all contributors to be consistent, so no one has doubt.

Thanks! Okay, all that's left here before we can issue payment and close this out is for @grgia to complete the BZ steps related to the PR that introduced the regression.

Not overdue.

@grgia bump for whenever you have a chance to complete the engineering BZ steps here! That's the last step before we can pay the reporting bonus here and close this out.

@joekaufmanexpensify thanks for the bump, I'll get this closed today

Of course, and great, ty!

Great, thanks Georgia!

@situchan $250 sent!

Closed upwork job.

Okay, bug is fixed, BZ checklist is complete, and payment is issued. Closing as this is all set. Thanks everyone!

authors: MelvinBot grgia joekaufmanexpensify s77rt situchan


text:
[HOLD - Notification project] Show local notification after file is downloaded from NewDot

Problem
Show a notification when files are downloaded from Expensify Chat.
Solution
This is a follow-up improvement to this issue -- which surfaces downloaded files, but drops the default OS notification.
After that issue is resolved, we should re-introduce the notification by creating one locally.
Upwork Automation - Do Not Edit

Upwork Job URL: https://www.upwork.com/jobs/~012a192beb41856435
Upwork Job ID: 1636392480505921536
Last Price Increase: 2023-03-16

Job added to Upwork: https://www.upwork.com/jobs/~012a192beb41856435

Triggered auto assignment to Contributor Plus for review of internal employee PR - @rushatgabhane (Internal)

Not required yet

Still low priority

authors: Julesssss MelvinBot


text:
[HOLD for payment 2023-05-16] [$1000] Jittery transition of tooltip when the inner content changes

If you haven’t already, check out our contributing guidelines for onboarding and email contributors@expensify.com to request to join our Slack channel!

Action Performed:

Navigate to any chat > Click on chat header
Click on the clipboard icon at the details page
Keep the cursor at clipboard and wait for the tooltip to change again to "Copy to clipboard"

Expected Result:
The tooltip transition is Jittery when the tooltip text changes.
Actual Result:
You shouldn't see any shaking/jitter in the transition.
Workaround:
Can the user still use Expensify without this being fixed? Have you informed them of the workaround?
Platforms:

Which of our officially supported platforms is this issue occurring on?

[ ] Android / native
[ ] Android / Chrome
[ ] iOS / native
[ ] iOS / Safari
[x] MacOS / Chrome / Safari
[x] MacOS / Desktop

Version Number: 1.3.1
Reproducible in staging?: y
Reproducible in production?: y
If this was caught during regression testing, add the test name, ID and link from TestRail:
Email or phone of affected tester (no customers):
Logs: https://stackoverflow.com/c/expensify/questions/4856
Notes/Photos/Videos: Any additional supporting documentation
https://user-images.githubusercontent.com/43996225/232654771-8182efd6-d634-40e9-b1b8-fc5f8a4d8efd.mp4
https://user-images.githubusercontent.com/43996225/232654786-3a83e4f3-19b9-4403-8df5-a8f864b32925.mov
Expensify/Expensify Issue URL:
Issue reported by: @thesahindia
Slack conversation: https://expensify.slack.com/archives/C049HHMV9SM/p1681726366513289
View all open jobs on GitHub
Upwork Automation - Do Not Edit

Upwork Job URL: https://www.upwork.com/jobs/~01b7a7c818708f281d
Upwork Job ID: 1651051677043404800
Last Price Increase: 2023-04-26

Triggered auto assignment to @michaelhaxhiu (Bug), see https://stackoverflow.com/c/expensify/questions/14418 for more details.

Bug0 Triage Checklist (Main S/O)

[ ] This "bug" occurs on a supported platform (ensure Platforms in OP are ✅)
[ ] This bug is not a duplicate report (check E/App issues and #expensify-bugs)

If it is, comment with a link to the original report, close the issue and add any novel details to the original issue instead

[ ] This bug is reproducible using the reproduction steps in the OP. S/O

If the reproduction steps are clear and you're unable to reproduce the bug, check with the reporter and QA first, then close the issue.
If the reproduction steps aren't clear and you determine the correct steps, please update the OP.

[ ] This issue is filled out as thoroughly and clearly as possible

Pay special attention to the title, results, platforms where the bug occurs, and if the bug happens on staging/production.

[ ] I have reviewed and subscribed to the linked Slack conversation to ensure Slack/Github stay in sync

@michaelhaxhiu Uh oh! This issue is overdue by 2 days. Don't forget to update your issues!

@michaelhaxhiu 6 days overdue. This is scarier than being forced to listen to Vogon poetry!

Job added to Upwork: https://www.upwork.com/jobs/~01b7a7c818708f281d

Current assignee @michaelhaxhiu is eligible for the External assigner, not assigning anyone new.

Triggered auto assignment to Contributor-plus team member for initial proposal review - @mananjadhav (External)

Triggered auto assignment to @amyevans (External), see https://stackoverflow.com/c/expensify/questions/7972 for more details.

Going external, this one is reproducible & valid!

Might be helpful
I think issue is caused here:
https://github.com/Expensify/App/blob/57388640eb97255c0089c4c316d13747b6a1dd34/src/components/Tooltip/TooltipRenderedOnPageBody.js#L101
When tooltip content changes we let the width be determined itself by setting width to undefined and immediately after that recalculate the width and provide it manually.

Proposal
Please re-state the problem that we are trying to solve in this issue.
Tooltip flickers when the content of the tooltip changes.
What is the root cause of that problem?
If we slow down the video, we can see that when the tooltip content changes, the position of the tooltip is late to update.

Currently, we position the tooltip based on the component width and the tooltip width itself. When the tooltip content changes, the onLayout callback will be called to update the new tooltip width.
https://github.com/Expensify/App/blob/6fb096948d862ee008445f809cff3ecd51b36f55/src/components/Tooltip/TooltipRenderedOnPageBody.js#L177-L178
https://github.com/Expensify/App/blob/6fb096948d862ee008445f809cff3ecd51b36f55/src/components/Tooltip/TooltipRenderedOnPageBody.js#L119-L124
As we can see, we will update the state inside onLayout callback, which could be late because the state is updated after the text content is updated.
content updated -> rendered -> onLayout -> width/height updated -> rendered
What changes do you think we should make in order to solve the problem?

Instead of relying on a fixed unit (tooltip width), we can use a percentage value to position the tooltip.

Remove tooltipWidth calculation from here
https://github.com/Expensify/App/blob/6fb096948d862ee008445f809cff3ecd51b36f55/src/styles/getTooltipStyles.js#L187

Add translateX -50% here (translateX should be put before the scale)
https://github.com/Expensify/App/blob/6fb096948d862ee008445f809cff3ecd51b36f55/src/styles/getTooltipStyles.js#L139-L146

Change the left to 50% and add transform: [{translateX: '-50%'}, {translateX: -horizontalShift}] here
https://github.com/Expensify/App/blob/6fb096948d862ee008445f809cff3ecd51b36f55/src/styles/getTooltipStyles.js#L222

https://user-images.githubusercontent.com/50919443/234776646-2ef430b4-0b37-4998-a6c1-7b0e461c2401.mov

I think @bernhardoj proposal looks good. We'll just have to check any regression absolute tooltips.
@amyevans all youts.

Just tested that my proposal won't cover the case when the tooltip is at the edge of the screen. That is because we still use tooltipWidth to calculate horizontalShift

Updated my proposal to cover all cases with useLayoutEffect

Niiiice one @bernhardoj thanks for being proactive on that. Let's get final 👍 from @amyevans and I'll proceed with assigning you.

Proposal looks great, assigning you @bernhardoj 🎉
We should be able to close out https://github.com/Expensify/App/issues/16211 as a byproduct of the bug fix as well 💪, 2 🐦 1 🥌

Will open the PR soon

PR is ready.

Thanks for the fast work @bernhardoj, next step is to get this reviewed by @mananjadhav

I've started with the review and going through the updated changes by @bernhardoj. Relatively larger than expected as we're changing the class component to functional component.

@amyevans I couldn't pinpoint to the exact PR causing this. We've been using measureTooltip since a long time. I am suspecting this PR must've caused it https://github.com/Expensify/App/pull/8494 but I couldn't confirm by running the specific branch.
Also, considering this isn't going to change that often, and we've moved to a functional component, I don't think we need a regression test here. @amyevans @michaelhaxhiu if you still feel we should add one, here's a proposal.
Regression Test Proposal

Open any chat
Click on their profile image of any participant
Hover over the Copy icon next to the user email/phone number
Verify a tooltip shows with a text "Copy to clipboard"
Click the Copy icon
Verify the tooltip text changes to "Copied"
Wait for a while and verify the text changes back to "Copy to clipboard"
Verify that the tooltip doesn't reposition when the text reverts.
Repeat the steps 5-8 few times to verify step 9 for each iteration.

I think this is not a regression as the initial implementation of the Tooltip has been using onLayout.
Btw, we have a regression (my bad) here https://github.com/Expensify/App/issues/18878#issuecomment-1550365511. I have found the issue and let me know if I should open the PR now.
cc: @mananjadhav @amyevans

Yeah @bernhardoj please open the PR.

PR is ready. https://github.com/Expensify/App/pull/19097

Please nudge me if I don't respond when it's time to process payment here (e.g. after the regression PR is merged and cleared)

Putting a date in the title that's more realistic, just so this doesn't appear behind.

Also, considering this isn't going to change that often, and we've moved to a functional component, I don't think we need a regression test here.

Agreed, I don't think we need to add a regression test for this.

@michaelhaxhiu Here's the last comment wrt to the Tooltip. This is ready for payout now, but also note it caused two regressions.

Actually, we still have 2 regression to be solved.

https://github.com/Expensify/App/issues/19842 - wrong height issue
https://expensify.slack.com/archives/C049HHMV9SM/p1685608239576139 - we should add preferred locale to tooltip key

Thanks for the insight! Agreed w/ @bernhardoj, let's hold payment until after those regressions are fully resolved.

I checked the PR. The changes look fine, but @michaelhaxhiu @kavimuru do we have a set of regression test suite that you can share wrt Tooltips? I just want to make sure we cover everything before we go ahead and merge this one?

Quick bump @michaelhaxhiu @kavimuru @amyevans if you can help with the previous comment?

hmm maybe we can ask @applausebot for their input as they interact the most with the test suites, I'm not sure off the top!

@kavimuru @applausebot Quick bump.

@isagoico recommended the following from our internal slack channel 🧠
We currently don't have tests to cover that tooltip. Imho we can add these tests to the TC that I shared above:
User detail page

Click on the header of the conversation
Verify there's a copy option next to the users email
Hover over the copy button
Verify a tooltip is displayed with the action ("Copy to Clipboard")
Click on the copy button
Verify the tooltip displays "Copied"

Thanks I think it won't cover all the regressions, may be with this issue we could update the Tooltip regressions along the way? I'll start with the given list and some that I am aware of.
On top of my head I can think of:

Tooltip doesn't flicker
Absolute Position of the tooltip works as expected
Copied resets to Copy to Clipboard after few seconds on Safari
Long Tooltip the arrows are aligned with the Bubble.
Multiline Tooltips show up without text being truncated
Switching between Pin/Unpin tooltip doesn't flicker and Unpin doesn't go multi-line.

@michaelhaxhiu I'll need a day to cover the testing for all the scenarios and still keeping 🤞. I don't want to add any more regressions 🙈

Since there was 2 regressions, the payout would be as follows:

Bug bonus $250 - @thesahindia
Contributor - $1000
C+ - $250

@michaelhaxhiu There's still another PR (https://github.com/Expensify/App/pull/20340) awaiting review/deployment/holding period so I'm not sure we're clear to proceed

ts to Copy to Clipboard after few seconds on Safari
Long Tooltip the arrows are aligned with the Bu

Hi there, these are all visual checks that are indirectly tested with the proposed steps here. We ask the testers to check for any visual issues when executing the steps without being explicitly told so in the step instructions. For context, we're avoiding having a extremely granular regression that checks every visual behavior of a feature and focusing mostly on functional behavior.
Going to add Copied resets to Copy to Clipboard after few seconds as this fits with the tests we want to cover.

@michaelhaxhiu we can add these 2 steps (7 & 8) to cover the button reset:
User detail page

Click on the header of the conversation
Verify there's a copy option next to the users email
Hover over the copy button
Verify a tooltip is displayed with the action ("Copy to Clipboard")
Click on the copy button
Verify the tooltip displays "Copied"
Web/Desktop - Wait a few seconds while hovering over the tooltip
Web/Desktop - Verify the tooltip changes back to "Copy" after a few seconds

wdyt?

I'll be testing this PR today.

@dukenv0307 yes i updated my prior comment to be more accurate.
We'll get this paid when the fix(es) are deployed to production.

Paid @mananjadhav on New Dot

@dukenv0307 can you share your upwork profile, or apply to the upword job directly?
https://www.upwork.com/jobs/~0139362b8c0838a40f

I invite @bernhardoj to the job just now.

Closest I could find the PR was https://github.com/Expensify/App/pull/8494 but I am not confident, hence I am not posting it on the PR. This is because I am not sure if we had Copy to Clipboard and we also made multiple changes in the component while fixing https://github.com/Expensify/App/issues/13146
I don't think this specific issue required a regression, but I think it makes sense to have a regression suite for Tooltip atleast? I have mentioned some cases here and we've got two additional cases here and here. @michaelhaxhiu Could you help with this? I am not sure if adding regression suite would be considered and what would be the steps here?

Re: the regression, I already made the regression test and linked it (yesterday). All set there.

Thanks @michaelhaxhiu.

@dukenv0307 lmk when you accept the job offer. Just need to pay you & this can be closed.

@michaelhaxhiu are we done with the payout? Can we close this one out?

paid now

authors: MelvinBot alitoshmatov amyevans anmurali bernhardoj isagoico kavimuru mananjadhav michaelhaxhiu


text:
PR:25564  LHN - Split is looped

If you haven’t already, check out our contributing guidelines for onboarding and email contributors@expensify.com to request to join our Slack channel!

Action Performed:

Launch App
Tap on FAB > Request money
Enter amount > Next
Select some users (3-4) tap on split
Tap on button "Add to split"

Expected Result:
Split chat for selected users should be appeared
Actual Result:
Request money page displayed again. Tap on Next clear amount and split is looped
Platforms:

Which of our officially supported platforms is this issue occurring on?

[x] Android / native
[ ] Android / Chrome
[ ] iOS / native
[ ] iOS / Safari
[ ] MacOS / Chrome / Safari
[ ] MacOS / Desktop

Version Number: 1.3.64-0 PR:25564
Reproducible in staging?: No
Reproducible in production?: No
If this was caught during regression testing, add the test name, ID and link from TestRail:
Email or phone of affected tester (no customers):
Logs: https://stackoverflow.com/c/expensify/questions/4856
Notes/Photos/Videos: Any additional supporting documentation
https://github.com/Expensify/App/assets/78819774/0fa7582f-1d0d-4bb0-b240-17987c287160
Expensify/Expensify Issue URL:
Issue reported by: Applause- Internal team
Slack conversation:
View all open jobs on GitHub

Link PR https://github.com/Expensify/App/pull/25564


text:
Deploy Checklist: New Expensify 2023-09-22

Release Version: 1.3.73-0
Compare Changes: https://github.com/Expensify/App/compare/production...staging
This release contains changes from the following pull requests:

[x] https://github.com/Expensify/App/pull/19345
[ ] https://github.com/Expensify/App/pull/23390
[x] https://github.com/Expensify/App/pull/23712
[x] https://github.com/Expensify/App/pull/24319
[x] https://github.com/Expensify/App/pull/24407
[ ] https://github.com/Expensify/App/pull/25416
[x] https://github.com/Expensify/App/pull/26029
[x] https://github.com/Expensify/App/pull/26236
[x] https://github.com/Expensify/App/pull/26543
[x] https://github.com/Expensify/App/pull/26710
[x] https://github.com/Expensify/App/pull/26727
[x] https://github.com/Expensify/App/pull/26861
[x] https://github.com/Expensify/App/pull/26934
[x] https://github.com/Expensify/App/pull/26969
[x] https://github.com/Expensify/App/pull/26973
[ ] https://github.com/Expensify/App/pull/26982
[x] https://github.com/Expensify/App/pull/27127
[x] https://github.com/Expensify/App/pull/27203
[x] https://github.com/Expensify/App/pull/27314
[x] https://github.com/Expensify/App/pull/27368
[x] https://github.com/Expensify/App/pull/27437
[x] https://github.com/Expensify/App/pull/27439
[ ] https://github.com/Expensify/App/pull/27459
[x] https://github.com/Expensify/App/pull/27468
[x] https://github.com/Expensify/App/pull/27475
[x] https://github.com/Expensify/App/pull/27505
[x] https://github.com/Expensify/App/pull/27506
[x] https://github.com/Expensify/App/pull/27550
[ ] https://github.com/Expensify/App/pull/27619
[x] https://github.com/Expensify/App/pull/27638
[x] https://github.com/Expensify/App/pull/27640
[x] https://github.com/Expensify/App/pull/27643
[x] https://github.com/Expensify/App/pull/27644
[x] https://github.com/Expensify/App/pull/27645
[x] https://github.com/Expensify/App/pull/27649
[x] https://github.com/Expensify/App/pull/27653
[x] https://github.com/Expensify/App/pull/27656
[x] https://github.com/Expensify/App/pull/27657
[x] https://github.com/Expensify/App/pull/27661
[x] https://github.com/Expensify/App/pull/27664
[x] https://github.com/Expensify/App/pull/27679
[x] https://github.com/Expensify/App/pull/27682
[x] https://github.com/Expensify/App/pull/27702
[x] https://github.com/Expensify/App/pull/27734
[ ] https://github.com/Expensify/App/pull/27739
[x] https://github.com/Expensify/App/pull/27740
[x] https://github.com/Expensify/App/pull/27743
[x] https://github.com/Expensify/App/pull/27749
[ ] https://github.com/Expensify/App/pull/27750
[x] https://github.com/Expensify/App/pull/27753
[x] https://github.com/Expensify/App/pull/27758
[x] https://github.com/Expensify/App/pull/27762
[ ] https://github.com/Expensify/App/pull/27765
[x] https://github.com/Expensify/App/pull/27769
[ ] https://github.com/Expensify/App/pull/27784
[x] https://github.com/Expensify/App/pull/27795
[x] https://github.com/Expensify/App/pull/27797
[ ] https://github.com/Expensify/App/pull/27808
[x] https://github.com/Expensify/App/pull/27834
[x] https://github.com/Expensify/App/pull/27840
[x] https://github.com/Expensify/App/pull/27842
[x] https://github.com/Expensify/App/pull/27845
[ ] https://github.com/Expensify/App/pull/27851
[x] https://github.com/Expensify/App/pull/27859
[x] https://github.com/Expensify/App/pull/27876
[x] https://github.com/Expensify/App/pull/27877
[x] https://github.com/Expensify/App/pull/27880
[x] https://github.com/Expensify/App/pull/27882
[x] https://github.com/Expensify/App/pull/27911
[x] https://github.com/Expensify/App/pull/27921
[x] https://github.com/Expensify/App/pull/27925
[ ] https://github.com/Expensify/App/pull/27932
[x] https://github.com/Expensify/App/pull/27945
[x] https://github.com/Expensify/App/pull/27946
[x] https://github.com/Expensify/App/pull/27955
[x] https://github.com/Expensify/App/pull/27957
[x] https://github.com/Expensify/App/pull/27969
[x] https://github.com/Expensify/App/pull/27978

Internal QA:

[ ] https://github.com/Expensify/App/pull/27844 - @youssef-lr

Deploy Blockers:

[ ] https://github.com/Expensify/App/issues/27094
[ ] https://github.com/Expensify/App/issues/27903
[ ] https://github.com/Expensify/App/issues/27928
[x] https://github.com/Expensify/App/issues/28033
[ ] https://github.com/Expensify/App/issues/28036
[ ] https://github.com/Expensify/App/issues/28042
[ ] https://github.com/Expensify/App/issues/28044
[ ] https://github.com/Expensify/App/issues/28057
[ ] https://github.com/Expensify/App/issues/28063
[x] https://github.com/Expensify/App/issues/28069
[x] https://github.com/Expensify/App/issues/28079
[x] https://github.com/Expensify/App/issues/28086
[ ] https://github.com/Expensify/App/issues/28092

Deployer verifications:

[ ] I checked the App Timing Dashboard and verified this release does not cause a noticeable performance regression.
[ ] I checked Firebase Crashlytics and verified that this release does not introduce any new crashes. More detailed instructions on this verification can be found here.
[ ] I checked GitHub Status and verified there is no reported incident with Actions.

cc @Expensify/applauseleads

:rocket: All staging deploys are complete, @Expensify/applauseleads please begin QA on version https://github.com/Expensify/App/releases/tag/1.3.73-0 :rocket:

https://github.com/Expensify/App/pull/27851 & https://github.com/Expensify/App/pull/27932 - PROD QA. Ok to check off?

Regression TR iOS 17 slots are left
#23390 failing with #28042
#24407 failing with #26136 repro in offline. Checking it off
#25416 failing with #28048
#26982 failing with #28046
#27459 failing with #28079
#27619 failing with #27365 (not fixed in mweb and iPhone)
#27739 failing with #28054 reproduced in production, ok to check it off?
#27750 failing with #28084  (As per this #27479 shouldn't be available- ok to check it off?)
#27784 -  could you check internally, Our accounts have silver wallet and this PR requires golden wallet with balance

@thienlnam Regression is completed.
#27765 failing with #28069
#27739 failing with #28054 in mweb
#23390 failing with #28042
#25416 failing with #28048
#26982 failing with #28046
#27459 failing with #28079
#27619 failing with #27365 (not fixed in mweb and iPhone)
#27739 failing with #28054 reproduced in production, ok to check it off?
#27750 failing with #28084 (As per this #27479 shouldn't be available- ok to check it off?)
#27784 - could you check internally, Our accounts have silver wallet and this PR requires golden wallet with balance

cc @chiragsalian As he is the App deployer this week

Checking off https://github.com/Expensify/App/issues/28086 as a blocker, it's very lower priority.

Checked off #28057 following cherry pick of fix and re-test in staging

Checking https://github.com/Expensify/App/pull/28089 off, verified internally

checking off:

https://github.com/Expensify/App/pull/27932
https://github.com/Expensify/App/pull/27851
https://github.com/Expensify/App/pull/27619

As these are related to ExpensiHelp page which is normally deployed on main and we can QA it separately and it should not be a staging blocker, this PR will resume it https://github.com/Expensify/App/pull/27988/
Checking off https://github.com/Expensify/App/pull/23390 as the found issue was demoted to normal bug
Checking https://github.com/Expensify/App/pull/26982 off as that is behind beta still'
checking https://github.com/Expensify/App/pull/27739 off as the found issues are in production/ not reproducible
checking https://github.com/Expensify/App/pull/27765 off as that is behind beta too
checking https://github.com/Expensify/App/pull/27459 off as that is behind categories beta
https://github.com/Expensify/App/pull/27784 verified internally https://expensify.slack.com/archives/C03TQ48KC/p1695630600830069
Checking https://github.com/Expensify/App/pull/25416 off as the issue found is not a blocker
Checking https://github.com/Expensify/App/pull/27750 as the save the world is not accessible yet

Graphs looking good
No new crashes standing out

:shipit:

authors: Julesssss OSBotify amyevans kavimuru mountiny mvtglobally thienlnam


text:
SAML SSO in NewDot - Desktop UI

Tracking GH issue: https://github.com/Expensify/Expensify/issues/294583
Relevant design doc section: https://docs.google.com/document/d/1xa68adybahTxucjSqvPJ2MZCZvaBoP1_vtYk5hTikGU/edit#heading=h.bkhju6ro8k33

Focusing on web / mweb first, then on platform specific logic

No updates, @NikkiWines what's the ETA on this one? Would it still make sense to ask Infinite Red to help with this one?

We're still trying to get web + mweb over the line but I'm hoping to start on it this week while I'm working 50%. I think it makes sense to open this up to contributors/agencies for all of the platform specific issues

According to @NikkiWines in Slack, Desktop was lumped into https://github.com/Expensify/App/pull/28372. @NikkiWines should we add this issue's link to the PR?

Yep!

See update here

No updates on this one.

This has the same PR as https://github.com/Expensify/App/issues/28215, no separate payment required.

This is done - desktop UI is live

authors: NikkiWines sakluger


text:
[$250] [Search v2.4] [App] Extend the 🔍icon and search router to all pages by adding “Search”

Implement this section of the doc
cc @Kicu @289Adam289 @Guccio163 @SzymczakJ
Issue OwnerCurrent Issue Owner: @
Upwork Automation - Do Not Edit

Upwork Job URL: https://www.upwork.com/jobs/~021834649747497603669
Upwork Job ID: 1834649747497603669
Last Price Increase: 2024-09-13

Hey! I’m Jakub Szymczak from Software Mansion, an expert agency, and I’d like to work on this issue!

@luacmartins Just confirming that this should be daily?

We can move it to weekly while we work on the first few issues to unblock this one.

I will start working on this task instead of @SzymczakJ, please reassign us

I believe this was deployed to production last week. @Kicu are you still working on anything else related to this issue?

Nope I don't think so, I'm only working on #49123 now

Summarizing payment on this issue:
Contributor: @Kicu - no payment required
Contributor+: @ikevin127 $250, paid via Upwork

@ntdiary I don't think we covered that in the doc and it's something we should address. @Kicu for your thoughts.

Hey, so first of all we treat this SearchRouter as a modal, so it's not a full fledged page (for example on web you will not see a dedicated URL for it).
So in theory this should not have much impact on navigation per se. It just appears when you open it - on top of whatever page was there, and then disappears after your interaction with it ends.
So if you open SearchRouter on Page X, then using either the back button or swipe should bring you back to Page X - I think this is the most natural transition.
So both 1 and 2 from @ntdiary comment sound like bugs that should be fixed. I think 🤔
Side note: calling @adamgrzybowski and @WojtekBoman who are working on navigation refactor, so it's possible this would be fixed on their PR, because the internal logic of screens, linkTo and back buttons will be changed after they are done.
On solution would be to wait, but I cannot guarantee how long this will take.
Not sure if that helps you a lot? 😓

@ntdiary sounds good to me.

authors: Kicu SzymczakJ luacmartins sakluger


text:
mwWeb - Chat - In edit comment, the cursor displayed area is not shown

If you haven’t already, check out our contributing guidelines for onboarding and email contributors@expensify.com to request to join our Slack channel!

Version Number: 9.0.61-0
Reproducible in staging?: Y
Reproducible in production?: Y
Issue reported by: Applause Internal Team
Action Performed:

Go to https://staging.new.expensify.com/home
Open a chat
Enter any message in compose and upload an image using plus icon left to chat input
Send the message
Long press the message sent and open edit comment
Note the cursor displayed area is shown
Tap enter key and enter any multi line message
Save the message
Long press the message sent and open edit comment

Expected Result:
In edit comment, the cursor displayed area must be shown.
Actual Result:
In edit comment, the cursor displayed area is not shown.
Workaround:
Unknown
Platforms:

[ ] Android: Standalone
[ ] Android: HybridApp
[x] Android: mWeb Chrome
[ ] iOS: Standalone
[ ] iOS: HybridApp
[ ] iOS: mWeb Safari
[ ] MacOS: Chrome / Safari
[ ] MacOS: Desktop

Screenshots/Videos
https://github.com/user-attachments/assets/77eefbbe-79bc-47e5-9605-b728b7488cbe

View all open jobs on GitHub

I don't really follow what the bug is based on the steps above. I also think the reproduction steps are very niche and not something a real user is likely to do. Closing this for now, let's revisit if a real customer reports this!

authors: IuliiaHerets joekaufmanexpensify


text:
Missing spanish translations in VBA flow (Company types and routing and bank account number example) - Reported by: @Santhosh-Sellavel

If you haven’t already, check out our contributing guidelines for onboarding and email contributors@expensify.com to request to join our Slack channel!

Action Performed:

Set app to spanish
Navigate to bank account flow > Manually set bank account
Check the image example above the routing and bank account number
Enter test credentials and proceed to company step
Click on the company type dropdown

Expected Result:
All page should be correctly translated to spanish.
Actual Result:
The example image in add bank account modal is not translated along with the company type options in Company step
Workaround:
None needed.
Platform:

Where is this issue occurring?

Web
iOS
Android
Desktop App
Mobile Web

Version Number: 1.1.8-8
Reproducible in staging?: Yes
Reproducible in production?: Yes
Logs: https://stackoverflow.com/c/expensify/questions/4856
Notes/Photos/Videos: Any additional supporting documentation

Expensify/Expensify Issue URL:
Issue reported by: @Santhosh-Sellavel
Slack conversation: https://expensify.slack.com/archives/C01GTK53T8Q/p1634748908001600
View all open jobs on GitHub

Triggered auto assignment to @flodnv (Engineering), see https://stackoverflow.com/c/expensify/questions/4319 for more details.

Assigned to @PrashantMangukiya
@shawnborton Can you answer Ioni's question above "Do you have that image?"

Apologies for the delay, here's the Spanish translation!

@michelle-thompson   thanks for the Spanish image. I will prepare and submit pr asap.

☝ PR deployed to staging

The solution for this issue has been :rocket: deployed to production :rocket: in version 1.1.14-4 and is now subject to a 7-day regression period :calendar:. Here is the list of pull requests that resolve this issue:

https://github.com/Expensify/App/pull/6190

If no regressions arise, payment will be issued on 2021-11-22. :confetti_ball:

@PrashantMangukiya paid!
@Santhosh-Sellavel can you please accept the offer for this job here so I can pay for reporting this issue? Thank you!

@NicMendonca Accepted!

@NicMendonca Contract is still open, end it when you have time thanks!

authors: MelvinBot NicMendonca PrashantMangukiya Santhosh-Sellavel botify isagoico mallenexpensify michelle-thompson


text:
Refactor Session.resendValidationLink and the ResendValidationForm to use the new API

Details

Fixed Issues

$ https://github.com/Expensify/Expensify/issues/211841
Tests

Use EmailOnDeck to register for a temporary email address.
Add the following line to ~/Web-Expensify/_config.local.php to whitelist this email address for dev.

define('DEV_EMAILS', ['<email address>']);

Navigate to the sign in page and enter this email address.
Click "Resend link". Verify that the following message appears when you do:

Now run vssh php /vagrant/Web-Expensify/script/notifyall.php to send out the validation link emails.
Verify that EmailOnDeck has received 2 emails with subject "Your magic sign in link for Expensify".

Now back on NewDot, click "Resend link" a bunch of times. Eventually, you'll get an error that should display like so:

PR Review Checklist

Contributor (PR Author) Checklist

[x] I linked the correct issue in the ### Fixed Issues section above
[x] I wrote clear testing steps that cover the changes made in this PR

[x] I added steps for local testing in the Tests section
[x] I added steps for Staging and/or Production testing in the QA steps section
[x] I added steps to cover failure scenarios (i.e. verify an input displays the correct error message if the entered data is not correct)
[x] I turned off my network connection and tested it while offline to ensure it matches the expected behavior (i.e. verify the default avatar icon is displayed if app is offline)

[ ] I included screenshots or videos for tests on all platforms
[ ] I ran the tests on all platforms & verified they passed on:

[ ] iOS / native
[ ] Android / native
[ ] iOS / Safari
[ ] Android / Chrome
[x] MacOS / Chrome
[ ] MacOS / Desktop

[x] I verified there are no console errors (if there's a console error not related to the PR, report it or open an issue for it to be fixed)
[x] I followed proper code patterns (see Reviewing the code)

[x] I verified that any callback methods that were added or modified are named for what the method does and never what callback they handle (i.e. toggleReport and not onIconClick)
[x] I verified that comments were added to code that is not self explanatory
[x] I verified that any new or modified comments were clear, correct English, and explained "why" the code was doing something instead of only explaining "what" the code was doing.
[x] I verified any copy / text shown in the product was added in all src/languages/* files
[x] I verified any copy / text that was added to the app is correct English and approved by marketing by tagging the marketing team on the original GH to get the correct copy.
[x] I verified proper file naming conventions were followed for any new files or renamed files. All non-platform specific files are named after what they export and are not named "index.js". All platform-specific files are named for the platform the code supports as outlined in the README.
[x] I verified the JSDocs style guidelines (in STYLE.md) were followed

[x] If a new code pattern is added I verified it was agreed to be used by multiple Expensify engineers
[x] I followed the guidelines as stated in the Review Guidelines
[x] I tested other components that can be impacted by my changes (i.e. if the PR modifies a shared library or component like Avatar, I verified the components using Avatar are working as expected)
[x] I verified all code is DRY (the PR doesn't include any logic written more than once, with the exception of tests)
[x] I verified any variables that can be defined as constants (ie. in CONST.js or at the top of the file that uses the constant) are defined as such
[x] If a new component is created I verified that:

[x] A similar component doesn't exist in the codebase
[x] All props are defined accurately and each prop has a /** comment above it */
[x] Any functional components have the displayName property
[x] The file is named correctly
[x] The component has a clear name that is non-ambiguous and the purpose of the component can be inferred from the name alone
[x] The only data being stored in the state is data necessary for rendering and nothing else
[x] For Class Components, any internal methods passed to components event handlers are bound to this properly so there are no scoping issues (i.e. for onClick={this.submit} the method this.submit should be bound to this in the constructor)
[x] Any internal methods bound to this are necessary to be bound (i.e. avoid this.submit = this.submit.bind(this); if this.submit is never passed to a component event handler like onClick)
[x] All JSX used for rendering exists in the render method
[x] The component has the minimum amount of code necessary for its purpose, and it is broken down into smaller components in order to separate concerns and functions

[x] If a new CSS style is added I verified that:

[x] A similar style doesn't already exist
[x] The style can't be created with an existing StyleUtils function (i.e. StyleUtils.getBackgroundAndBorderStyle(themeColors.componentBG)

[x] If the PR modifies a generic component, I tested and verified that those changes do not break usages of that component in the rest of the App (i.e. if a shared library or component like Avatar is modified, I verified that Avatar is working as expected in all cases)
[x] If the PR modifies a component related to any of the existing Storybook stories, I tested and verified all stories for that component are still working as expected.
[x] I have checked off every checkbox in the PR author checklist, including those that don't apply to this PR.

PR Reviewer Checklist
The Contributor+ will copy/paste it into a new comment and complete it after the author checklist is completed

[ ] I have verified the author checklist is complete (all boxes are checked off).
[ ] I verified the correct issue is linked in the ### Fixed Issues section above
[ ] I verified testing steps are clear and they cover the changes made in this PR

[ ] I verified the steps for local testing are in the Tests section
[ ] I verified the steps for Staging and/or Production testing are in the QA steps section
[ ] I verified the steps cover any possible failure scenarios (i.e. verify an input displays the correct error message if the entered data is not correct)
[ ] I turned off my network connection and tested it while offline to ensure it matches the expected behavior (i.e. verify the default avatar icon is displayed if app is offline)

[ ] I checked that screenshots or videos are included for tests on all platforms
[ ] I verified tests pass on all platforms & I tested again on:

[ ] iOS / native
[ ] Android / native
[ ] iOS / Safari
[ ] Android / Chrome
[ ] MacOS / Chrome
[ ] MacOS / Desktop

[ ] I verified there are no console errors (if there's a console error not related to the PR, report it or open an issue for it to be fixed)
[ ] I verified proper code patterns were followed (see Reviewing the code)

[ ] I verified that any callback methods that were added or modified are named for what the method does and never what callback they handle (i.e. toggleReport and not onIconClick).
[ ] I verified that comments were added to code that is not self explanatory
[ ] I verified that any new or modified comments were clear, correct English, and explained "why" the code was doing something instead of only explaining "what" the code was doing.
[ ] I verified any copy / text shown in the product was added in all src/languages/* files
[ ] I verified any copy / text that was added to the app is correct English and approved by marketing by tagging the marketing team on the original GH to get the correct copy.
[ ] I verified proper file naming conventions were followed for any new files or renamed files. All non-platform specific files are named after what they export and are not named "index.js". All platform-specific files are named for the platform the code supports as outlined in the README.
[ ] I verified the JSDocs style guidelines (in STYLE.md) were followed

[ ] If a new code pattern is added I verified it was agreed to be used by multiple Expensify engineers
[ ] I verified that this PR follows the guidelines as stated in the Review Guidelines
[ ] I verified other components that can be impacted by these changes have been tested, and I retested again (i.e. if the PR modifies a shared library or component like Avatar, I verified the components using Avatar have been tested & I retested again)
[ ] I verified all code is DRY (the PR doesn't include any logic written more than once, with the exception of tests)
[ ] I verified any variables that can be defined as constants (ie. in CONST.js or at the top of the file that uses the constant) are defined as such
[ ] If a new component is created I verified that:

[ ] A similar component doesn't exist in the codebase
[ ] All props are defined accurately and each prop has a /** comment above it */
[ ] Any functional components have the displayName property
[ ] The file is named correctly
[ ] The component has a clear name that is non-ambiguous and the purpose of the component can be inferred from the name alone
[ ] The only data being stored in the state is data necessary for rendering and nothing else
[ ] For Class Components, any internal methods passed to components event handlers are bound to this properly so there are no scoping issues (i.e. for onClick={this.submit} the method this.submit should be bound to this in the constructor)
[ ] Any internal methods bound to this are necessary to be bound (i.e. avoid this.submit = this.submit.bind(this); if this.submit is never passed to a component event handler like onClick)
[ ] All JSX used for rendering exists in the render method
[ ] The component has the minimum amount of code necessary for its purpose, and it is broken down into smaller components in order to separate concerns and functions

[ ] If a new CSS style is added I verified that:

[ ] A similar style doesn't already exist
[ ] The style can't be created with an existing StyleUtils function (i.e. StyleUtils.getBackgroundAndBorderStyle(themeColors.componentBG)

[ ] If the PR modifies a generic component, I tested and verified that those changes do not break usages of that component in the rest of the App (i.e. if a shared library or component like Avatar is modified, I verified that Avatar is working as expected in all cases)
[ ] If the PR modifies a component related to any of the existing Storybook stories, I tested and verified all stories for that component are still working as expected.
[ ] I have checked off every checkbox in the PR reviewer checklist, including those that don't apply to this PR.

QA Steps
Same as the testing steps, except you can skip steps 2 and 6.

[ ] Verify that no errors appear in the JS console

Screenshots

Web

Mobile Web

Desktop

iOS

Android

@shawnborton picking up from this:

I feel like this should be extrapolated out into a reusable component, and should build off of what we already have in place for the red brick road message.

Where is this red brick road message located?

Here is some good reading: https://github.com/Expensify/App/pull/9931

Here is some good reading: #9931

@shawnborton I see! That component is intended to be used with Pattern B, but the resend validation link flow is actually Pattern C, so this component isn't entirely reusable.
What I'll do is refactor the red brick/text styles from the OfflineWithFeedback component that was added in that PR into another component (name suggestions welcome), and reuse that component in both the OfflineWithFeedback component and the ResendValidationForm. Thoughts?

@shawnborton Ended up creating a DotIndicatorMessage component, since it can be used for both errors and success messages. Please let me know what you think!

@shawnborton @bondydaa @aldo-expensify Updates the screenshots in the OP to include the new dot indicator message styles, ready for another review, thanks in advance :)

@iwiznia Requesting you for review as well since I touched the OfflineWithFeedback component you implemented recently.

I tested in Android device and the styles are looking good for the error/success message

That component is intended to be used with Pattern B, but the resend validation link flow is actually Pattern C, so this component isn't entirely reusable.

I wouldn't think of this as being a matter of which pattern we're using. But rather, we have a standard "brick road message" component that is used across various offline patterns, form patterns, etc. I think you totally got the point though, but just wanted to reiterate that. But thanks for hearing me out and turning this into a component! Can you also add that to Storybook? Or does that happen automatically?

Hey guys! Was caught up working on https://github.com/Expensify/App/pull/10235 today since it's been open for a while ad the Web PR it relied on was recently deployed to production. I'm planning on circling back to this first thing next week.

@bondydaa @aldo-expensify good for another look!

@bondydaa @iwiznia Thank you guys for the thorough review! I've responded/addressed your review comments and we should be good for another look :)

Tested in Android, looking good.

updated again, here are more screenshots of desktop

@bondydaa cool if I assign you this PR? that you also get contributions (I think) when it gets merged.

that way you also get contributions (I think) when it gets merged

contributions are about commits not who's assigned so we'll both get them for the commits that we author'd (or co-author'd) that get merged.  😉

ready for another review 🙏

:hand: This PR was not deployed to staging yet because QA is ongoing. It will be automatically deployed to staging after the next production release.

🚀 Deployed to staging by @bondydaa in version: 1.1.96-0 🚀

platform
result

🤖 android 🤖
success ✅

🖥 desktop 🖥
success ✅

🍎 iOS 🍎
success ✅

🕸 web 🕸
success ✅

This PR might have caused this regression.

I think you linked the wrong thing on this regression but I see https://github.com/Expensify/App/issues/13824 popping up underneath your comment so figured it out.
Yeah I agree this was probably the cause, we should always be sure to test with 0, 1, many workspace members (and errors) to ensure the border is displayed properly.

I think you linked the wrong thing on this regression

Oops my mistake. Updated the comment.

authors: OSBotify aldo-expensify bondydaa jasperhuangg shawnborton sobitneupane


text:
Create script to aggregate GitHub data

Details

Fixed Issues

$ GH_LINK
PROPOSAL: GH_LINK_ISSUE(COMMENT)
Tests

[ ] Verify that no errors appear in the JS console

Offline tests

QA Steps

[ ] Verify that no errors appear in the JS console

PR Author Checklist

[ ] I linked the correct issue in the ### Fixed Issues section above
[ ] I wrote clear testing steps that cover the changes made in this PR

[ ] I added steps for local testing in the Tests section
[ ] I added steps for the expected offline behavior in the Offline steps section
[ ] I added steps for Staging and/or Production testing in the QA steps section
[ ] I added steps to cover failure scenarios (i.e. verify an input displays the correct error message if the entered data is not correct)
[ ] I turned off my network connection and tested it while offline to ensure it matches the expected behavior (i.e. verify the default avatar icon is displayed if app is offline)
[ ] I tested this PR with a High Traffic account against the staging or production API to ensure there are no regressions (e.g. long loading states that impact usability).

[ ] I included screenshots or videos for tests on all platforms
[ ] I ran the tests on all platforms & verified they passed on:

[ ] Android / native
[ ] Android / Chrome
[ ] iOS / native
[ ] iOS / Safari
[ ] MacOS / Chrome / Safari
[ ] MacOS / Desktop

[ ] I verified there are no console errors (if there's a console error not related to the PR, report it or open an issue for it to be fixed)
[ ] I followed proper code patterns (see Reviewing the code)

[ ] I verified that any callback methods that were added or modified are named for what the method does and never what callback they handle (i.e. toggleReport and not onIconClick)
[ ] I verified that comments were added to code that is not self explanatory
[ ] I verified that any new or modified comments were clear, correct English, and explained "why" the code was doing something instead of only explaining "what" the code was doing.
[ ] I verified any copy / text shown in the product is localized by adding it to src/languages/* files and using the translation method

[ ] If any non-english text was added/modified, I verified the translation was requested/reviewed in #expensify-open-source and it was approved by an internal Expensify engineer. Link to Slack message:

[ ] I verified all numbers, amounts, dates and phone numbers shown in the product are using the localization methods
[ ] I verified any copy / text that was added to the app is correct English and approved by marketing by adding the Waiting for Copy label for a copy review on the original GH to get the correct copy.
[ ] I verified proper file naming conventions were followed for any new files or renamed files. All non-platform specific files are named after what they export and are not named "index.js". All platform-specific files are named for the platform the code supports as outlined in the README.
[ ] I verified the JSDocs style guidelines (in STYLE.md) were followed

[ ] If a new code pattern is added I verified it was agreed to be used by multiple Expensify engineers
[ ] I followed the guidelines as stated in the Review Guidelines
[ ] I tested other components that can be impacted by my changes (i.e. if the PR modifies a shared library or component like Avatar, I verified the components using Avatar are working as expected)
[ ] I verified all code is DRY (the PR doesn't include any logic written more than once, with the exception of tests)
[ ] I verified any variables that can be defined as constants (ie. in CONST.js or at the top of the file that uses the constant) are defined as such
[ ] I verified that if a function's arguments changed that all usages have also been updated correctly
[ ] If a new component is created I verified that:

[ ] A similar component doesn't exist in the codebase
[ ] All props are defined accurately and each prop has a /** comment above it */
[ ] The file is named correctly
[ ] The component has a clear name that is non-ambiguous and the purpose of the component can be inferred from the name alone
[ ] The only data being stored in the state is data necessary for rendering and nothing else
[ ] For Class Components, any internal methods passed to components event handlers are bound to this properly so there are no scoping issues (i.e. for onClick={this.submit} the method this.submit should be bound to this in the constructor)
[ ] Any internal methods bound to this are necessary to be bound (i.e. avoid this.submit = this.submit.bind(this); if this.submit is never passed to a component event handler like onClick)
[ ] All JSX used for rendering exists in the render method
[ ] The component has the minimum amount of code necessary for its purpose, and it is broken down into smaller components in order to separate concerns and functions

[ ] If any new file was added I verified that:

[ ] The file has a description of what it does and/or why is needed at the top of the file if the code is not self explanatory

[ ] If a new CSS style is added I verified that:

[ ] A similar style doesn't already exist
[ ] The style can't be created with an existing StyleUtils function (i.e. StyleUtils.getBackgroundAndBorderStyle(themeColors.componentBG))

[ ] If the PR modifies code that runs when editing or sending messages, I tested and verified there is no unexpected behavior for all supported markdown - URLs, single line code, code blocks, quotes, headings, bold, strikethrough, and italic.
[ ] If the PR modifies a generic component, I tested and verified that those changes do not break usages of that component in the rest of the App (i.e. if a shared library or component like Avatar is modified, I verified that Avatar is working as expected in all cases)
[ ] If the PR modifies a component related to any of the existing Storybook stories, I tested and verified all stories for that component are still working as expected.
[ ] If a new page is added, I verified it's using the ScrollView component to make it scrollable when more elements are added to the page.
[ ] If the main branch was merged into this PR after a review, I tested again and verified the outcome was still expected according to the Test steps.
[ ] I have checked off every checkbox in the PR author checklist, including those that don't apply to this PR.

Screenshots/Videos

Web

Mobile Web - Chrome

Mobile Web - Safari

Desktop

iOS

Android

Assuming we will move this to a different repo, but pushed final code here.
Output: https://expensify.slack.com/archives/C03TUCV48/p1681410039827329?thread_ts=1681399272.393779&cid=C03TUCV48

So where do we think this script should live long-term then? Will we even need it?
I kind of think a better long-term plan would be to export the cost center into Upwork along with the job listing so we can get all the data we need in one place (Upwork)

I would think this could be it's own repo under Expensify org.

Let's move this to a separate repo under Expensify internal

It's not here to stay. It will be replaced by a supportal tool: https://docs.google.com/document/d/1NrhP_sbZ40cVKSiSUXj-6o1w-hfH_rv8miRI57WId5M/edit#heading=h.ns12dr1f35rx

Closing this out. It was never meant to be merged, and should not be needed anymore now that we have a supportal tool for this

authors: AndrewGable roryabraham