[Details Revamp] Update ReportDetailsPage for Rooms, Groups, Money Reports and Policy Expenses

Details

Update for the ReportDetailsPage for all the possible use-cases, such as Rooms and Groups, Money Reports/Requests and Policy Expense Report/Requests.
Fixed Issues

$ https://github.com/Expensify/App/issues/42074
$ https://github.com/Expensify/App/issues/42075
$ https://github.com/Expensify/App/issues/42076
PROPOSAL:
Tests

Workspace Rooms & Groups:

Create a new workspace, inside it create a custom room and a chat group with at least two people
Go into the newly created rooms, and check for correctness of the following parameters:
If the room is not default and the user should be able to leave it, the leave button should appear and be interactable
The PromotedActionsBar should be showing the Pin and Share actions, Join if the user is a guest.
The name should be displayed in a new way, editable when the rooms aren't the default rooms.
Under the name field there should be a small subtext that will either redirect you to the workspace page if you are an admin of said workspace, or just a text displaying the name of the workspace you are a member of.

IOU / Expense Reports:

Create a new IOU / Expense report.
If the expense is deleteable, the delete button should appear and display the confirmation modal through which you can delete the selected expense.
On any of the expenses there should be the PromotedActionsBar displaying the correct values, which are Pin and Share actions, Hold if the expense is available for being held.

[x] Verify that no errors appear in the JS console

Offline tests

QA Steps

[x] Verify that no errors appear in the JS console

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 the expected offline behavior in the Offline steps 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)
[x] 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).

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

[x] Android: Native
[x] Android: mWeb Chrome
[x] iOS: Native
[x] iOS: mWeb Safari
[x] MacOS: Chrome / Safari
[x] 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 the left part of a conditional rendering a React component is a boolean and NOT a string, e.g. myBool && <MyComponent />.
[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 is localized by adding it to src/languages/* files and using the translation method

[x] 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:

[x] I verified all numbers, amounts, dates and phone numbers shown in the product are using the localization methods
[x] I verified any copy / text that was added to the app is grammatically correct in English. It adheres to proper capitalization guidelines (note: only the first word of header/labels should be capitalized), and is either coming verbatim from figma or has been approved by marketing (in order to get marketing approval, ask the Bug Zero team member to add the Waiting for copy label to the issue)
[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] I verified that if a function's arguments changed that all usages have also been updated correctly
[x] If any new file was added I verified that:

[x] 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

[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(theme.componentBG))

[x] 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.
[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] If the PR modifies a component or page that can be accessed by a direct deeplink, I verified that the code functions as expected when the deeplink is used - from a logged in and logged out account.
[x] If the PR modifies the UI (e.g. new buttons, new UI components, changing the padding/spacing/sizing, moving components, etc) or modifies the form input styles:

[x] I verified that all the inputs inside a form are aligned with each other.
[x] I added Design label and/or tagged @Expensify/design so the design team can review the changes.

[x] 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.
[x] 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.

Screenshots/Videos

Android: Native

Android: mWeb Chrome

iOS: Native

iOS: mWeb Safari

MacOS: Chrome / Safari

MacOS: Desktop

As an update, during the creation of test steps I've encountered a case that isn't yet handled. I'm currently adding it into the page after which this PR will be able to be moved to review.

@kosmydel Added the remaining case and test steps, ready for internal review.

BUG? The Delete button should be displayed?
https://github.com/Expensify/App/assets/104823336/ebe09e5b-89ed-4de1-b44c-4cb010f1e402

BUG: Empty settings
https://github.com/Expensify/App/assets/104823336/0709d209-4546-4ba4-bbee-8535a68a7c95

TS checks are failing cc @cdOut

BUG: Delete comment instead of Delete expense

BUG? Maybe not related to this PR, but when I delete an expense in RHP I get some infinity loader.

BUGs 1, 2 are fixed, 3 and 4 both have the same root cause and it has to do with contexts of reports, parentReports and reportActions that has changed after migrating some functionality from Report Headers into this Page. I am currently looking for a solution to it.

Reviewer Checklist

[ ] 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 included screenshots or videos for tests on all platforms
[ ] I verified tests pass on all platforms & I tested again on:

[ ] Android: Native
[ ] Android: mWeb Chrome
[ ] iOS: Native
[ ] iOS: mWeb Safari
[ ] MacOS: Chrome / Safari
[ ] MacOS: Desktop

[ ] If there are any errors in the console that are unrelated to this PR, I either fixed them (preferred) or linked to where I reported them in Slack
[ ] 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 the left part of a conditional rendering a React component is a boolean and NOT a string, e.g. myBool && <MyComponent />.
[ ] 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
[ ] 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 grammatically correct in English. It adheres to proper capitalization guidelines (note: only the first word of header/labels should be capitalized), and is either coming verbatim from figma or has been approved by marketing (in order to get marketing approval, ask the Bug Zero team member to add the Waiting for copy label to the issue)
[ ] 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 */
[ ] 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(theme.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 the PR modifies a component or page that can be accessed by a direct deeplink, I verified that the code functions as expected when the deeplink is used - from a logged in and logged out account.
[ ] If the PR modifies the UI (e.g. new buttons, new UI components, changing the padding/spacing/sizing, moving components, etc) or modifies the form input styles:

[ ] I verified that all the inputs inside a form are aligned with each other.
[ ] I added Design label and/or tagged @Expensify/design so the design team can review the changes.

[ ] 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 reviewer checklist, including those that don't apply to this PR.

Screenshots/Videos

Android: Native

Android: mWeb Chrome

iOS: Native

iOS: mWeb Safari

MacOS: Chrome / Safari

MacOS: Desktop

I've fixed merge conflicts and updated main.

@cdOut Could you please add video or screenshots in the PR description at least for MacOS: Chrome so I can get an idea of how the new changes should look ?
I'm asking because I don't have access to the design doc and have no idea of how the new UI should look or what to look for to confirm that the changes occured (besides the testing steps), and some visual feedback would help a lot.

@ikevin127 I've added some temporary screenshots of how the screens should look like for specific cases, I will supply test videos for the rest of the platforms tomorrow.

@cdOut Thank you! I'll start working on the checklist.

@ikevin127 Changes in this PR are pretty complicated. I think that you should have access to the design doc.

@kosmydel @cdOut Could any of you clone the design docs and share them with me on<EMAIL_ADDRESS>?
I signed the NDA so there shouldn't be any issues, though if this is not possible, I already requested access with the same email to all 3 docs mentioned in issue's OP.
cc @grgia

@cdOut I took a look at the PR at point based on Test steps and attached screenshots and I found the following:
♻  Workspace Rooms & Groups
⚠ Potential UI / UX Issues

Group RHP - Weird padding element compared to staging

PR
Staging

Room RHP - No spacing between Room name field and WS subtext (visible when highlighted)

Note: This was probably done on purpose to match the distance between the Room name label and value (see non-highlighted picture below), but I thought I should mention it so we can validate it with @Expensify/design team.

PR
Staging

⚠ Potential Functionality Issue

Room Guest Header - Showing both `Join` button and Join option in 3-dot popover

♻ IOU / Expense Reports
⚠ Potential UI / UX Issues

WS Expense Report RHP - More spacing between Expense Report name and actions bar compared to staging

PR
Staging

WS Expense Report RHP - [Delete comment] label is incorrect since the button performs [Delete expense]

Note: The label doesn't align with what the button is doing since it's not deleting a comment but an expense (see modal).

⚠ Potential Functionality Issue

WS Expense Report RHP - The [Hold] is showing in the RHP when it shouldn't
Scenario 1

Submit only 1 (one) expense in a new WS chat.
Navigate to the expense by clicking on the WS chat expense preview card.
Click on 3-dot menu in the top-right side of the expense report header and verify that the Hold option is NOT present.
Click on the expense report header to open RHP and notice that the Hold action is SHOWING even though it shouldn't.

Scenario 2

Submit 2 (two) expenses in a new WS chat.
Navigate to the expense by clicking on the WS chat expense preview card which shows 2 expenses subtext.
You will be navigated to another preview (with cards) where you will see the 2 expenses separately.
Without clicking on any of the two expenses card, click on 3-dot menu in the top-right side of the expense report header and verify that the Hold option is NOT present.
Click on the expenses (plural) report header to open RHP and notice that the Hold action is SHOWING even though it shouldn't since this is not any of the 2 individual expenses RHP panel.

Important: Note that the Hold action should only show-up in the RHP of an expense when that expense can actually by held, basically whenever this option is being shown in the 3-dot menu on current staging build.

Scenario 1
Scenario 2

I noted the issues above as potential because I need confirmation from PR author and docs (once I get access) to determine whether they are actual issues that would create regressions or all is good and they are meant to be like that.

[!note]
Additionally, I think @Expensify/design should take a look at some of the small UI/UX potential issues and confirm how we want things to look.

Can you tell me what the overlap is between this PR here, and this PR that @c3024  is working on? Seems like there is a lot of overlap, and I'm wondering how much of it is being addressed in @c3024 's PR.

Group RHP - Weird padding element compared to staging

Seems fine to me I think, given that we now have a push row underneath.

Room RHP - No spacing between Room name field and WS subtext (visible when highlighted)

This seems broken, but in the PR I linked, it seems to work just fine (hint text is included in hover)

Room Guest Header - Showing both Join button and Join option in 3-dot popover

I thought the idea was to get rid of the three dot button everywhere? cc @grgia

WS Expense Report RHP - More spacing between Expense Report name and actions bar compared to staging

Looks correct to me, but again, it should be good in that linked PR.

WS Expense Report RHP - [Delete comment] label is incorrect since the button performs [Delete expense]

Yeah, let's fix that.

Room Guest Header - Showing both Join button and Join option in 3-dot popover

There is a follow up issue in Details Revamp that will focus on removing all the 3-dot menus, removing them is not a part of this one.

Can you tell me what the overlap is between this PR here, and this PR that @c3024 is working on? Seems like there is a lot of overlap, and I'm wondering how much of it is being addressed in @c3024 's PR.

There is a lot of overlap, but the linked PR is only concerned with moving the name section out of Settings into the ReportDetailsPage, which is just one of a few changes that had to be made for the issues handled in this PR. The linked PR seems to also be solving an issue that predates the Details Revamp refactor, I wasn't made aware of this one and this was also one of the things I needed to do as part of my issues.
I'll unify the code with what is in the PR linked by Shawn, what's the further steps here @grgia? Should there be a clear order of when we merge this and the linked PR?
The rest of the listed issues I will work on fixing.

@ikevin127 I suggest we wait for someone from Expensify to share the design doc with you, as I'm not sure what's the policy.

I noticed the 2 identical early returns while looking at the code of this PR and maybe we can remove one of them here

Thanks for noticing! @cdOut could you remove the redundant early return in this PR?

Thanks for noticing! @cdOut could you remove the redundant early return in this PR?

Of course, I've removed the redundant early return.

I'll unify the code with what is in the https://github.com/Expensify/App/pull/43636, what's the further steps here @grgia? Should there be a clear order of when we merge this and the linked PR?

Sounds like this is a plan. I don't think the order of merging matters too much here, do you have a preference?  @cdOut

Sounds like this is a plan. I don't think the order of merging matters too much here, do you have a preference?

Don't really have a preference however the person working on this feature should be notified and have this in mind if this PR gets merged before his.

@cdOut Could you please let me know based on latest discussions and the potential issues listed in https://github.com/Expensify/App/pull/43251#issuecomment-2166899220:

which one of the issues are actual issues that will be addressed here
which ones are non-issues that don't require changes

Asking because I want to know where we are with that list in order for me to be able to start working on the checklist.

@grgia I requested access with<EMAIL_ADDRESS>to all 3 docs from the issues attached in this PR's Fixed Issues section ? As recommended by https://github.com/Expensify/App/pull/43251#issuecomment-2166557345 so I can start working on the reviewer checklist once I get a response regarding the list mentioned above.

I've bumped @grgia to give you access to the design doc 😇

@ikevin127 of course, here's the rundown of issues you've mentioned:

Group RHP - Weird padding element compared to staging

This is an issue, will be fixed within the next commits after unifying the name field with the linked PR.

Room RHP - No spacing between Room name field and WS subtext (visible when highlighted)

This is an issue, will be fixed within the next commits after unifying the name field with the linked PR.

Room Guest Header - Showing both Join button and Join option in 3-dot popover

This is not an issue for this PR, all the 3-dot menus will be removed in a follow-up PR.

WS Expense Report RHP - More spacing between Expense Report name and actions bar compared to staging

This is an issue, will be fixed within the next commits after unifying the name field with the linked PR.

WS Expense Report RHP - [Delete comment] label is incorrect since the button performs [Delete expense]

This is an issue, will be fixed within the next commits.

WS Expense Report RHP - The [Hold] is showing in the RHP when it shouldn't

This is an issue, will be fixed within the next commits.

@ikevin127 I shared the Details Revamp Doc with you for PR review.

@grgia I wanted to clear some things when it comes to handling Tasks. Currently in the design doc there is no mention of them, even though the revamp does affect screens that appear when dealing with user created tasks in the app. Should the delete button / promoted actions be customized for tasks also?
https://github.com/Expensify/App/assets/88325488/ba76d26e-78cb-471a-997d-aab81550b843

@cdOut yes, we should use the new pattern globally

@cdOut yes, we should use the new pattern globally

@grgia I believe we should include this in the pull request as it is closely related. However, incorporating and testing it will take some time, as we have just discovered this case.
Is that okay, or should we proceed with this pull request and create a new one for the task change?
For now, I think we should make this PR a draft, and add the additional changes.
cc @cdOut

Here's a list of the currently found issues, it will be updated whenever we deal with one of the listed items.

[x] Group RHP - Weird padding element compared to staging
[x] Room RHP - No spacing between Room name field and WS subtext (visible when highlighted)
[x] WS Expense Report RHP - More spacing between Expense Report name and actions bar compared to staging
[x] WS Expense Report RHP - [Delete comment] label is incorrect since the button performs [Delete expense]
[ ] WS Expense Report - Delete action is processed incorrectly in some specific cases
[ ] WS Expense Report RHP - The [Hold] is showing in the RHP when it shouldn't
[ ]  Tasks RHP - Include this case in the revamp

cc: @grgia @Expensify/design
Since task weren't considered in the design doc, where should that action fall? Should there be a separate MenuItem for it or should this be placed in the PromotedActionsBar (The Mark as Incomplete action for tasks)?

Brought to slack @cdOut https://expensify.slack.com/archives/C036QM0SLJK/p1718627217374219

Posted in slack with my thoughts!

@cdOut Thanks for the answers in https://github.com/Expensify/App/pull/43251#issuecomment-2172562654! Please let me know once the remaining items from https://github.com/Expensify/App/pull/43251#issuecomment-2173144100 are addressed so I can start working on the checklist.

@grgia I think so, @ikevin127 the PR should be ready to be retested, we've fixed the previous issues and implemented handling the Tasks case as well.

The only remaining issue from the previous list in https://github.com/Expensify/App/pull/43251#issuecomment-2166899220 is this spacing related discrepancy:
♻ IOU / Expense Reports
⚠ Potential UI / UX Issues

WS Expense Report RHP - More spacing between Expense Report name and actions bar compared to staging

PR
Staging

Clarifications

Regarding Tasks, I noticed  the commit feat: add markAsIncomplete. Given this, and the fact that the Details Revamp Doc doesn't mention Tasks, can the PR author add test steps in the PR description for the Tasks related changes ? I need to know what to look for because I cannot access the Slack discussion from #43251 (comment).
Taking note of the Details Revamp Doc -> Plan of action / Rollout plan section (see picture below) and looking at the PRs code changes, I did not notice changes related to point 7. Update ProfilePage and 7. Remove DetailsPage. Are we supposed to include thosw 2 in this PR as well or we're skipping them ?

In the meantime I will proceed with working on the checklist.

Clarifications

Regarding Tasks, I noticed  the commit feat: add markAsIncomplete. Given this, and the fact that the Details Revamp Doc doesn't mention Tasks, can the PR author add test steps in the PR description for the Tasks related changes ? I need to know what to look for because I cannot access the Slack discussion from #43251 (comment).
Taking note of the Details Revamp Doc -> Plan of action / Rollout plan section (see picture below) and looking at the PRs code changes, I did not notice changes related to point 7. Update ProfilePage and 7. Remove DetailsPage. Are we supposed to include thosw 2 in this PR as well or we're skipping them ?

⚠ PR vs Details Revamp Doc Discrepancy
Doc reference -> CLICK

[!tip]

As member of a workspace go to your workspace chat (between you and the workspace).
Create and submit 2 manual expenses.
As admin of the workspace navigate to the member's chat with the workspace.
Click on the expenses (plural) report preview in order to see 2 separate report previews for each expense.
Click on one of the expenses report preview.
Click on the 3-dot menu in the top-right side of the expense report header and notice that the Hold option is present.
Click on expense report header to open RHP.
Click on the expense report header to open RHP and notice that the Hold button is NOT showing in Promoted Actions even though it should since it is showing in the 3-dot menu because WS admin should be able to hold WS member's expenses.

WS Expense Report RHP - Hold button not present in Promoted Actions for WS Admin

PR
Doc

Regarding Tasks, I noticed  the commit feat: add markAsIncomplete. Given this, and the fact that the Details Revamp Doc doesn't mention Tasks, can the PR author add test steps in the PR description for the Tasks related changes ? I need to know what to look for because I cannot access the Slack discussion from #43251 (comment).

Of course, give me a moment and I'll add the test steps for Tasks case.

Taking note of the Details Revamp Doc -> Plan of action / Rollout plan section (see picture below) and looking at the PRs code changes, I did not notice changes related to point 7. Update ProfilePage and 7. Remove DetailsPage. Are we supposed to include thosw 2 in this PR as well or we're skipping them ?

These were already handled in separate PRs, which you can see here and here.
I've already corrected the other two issues you've pointed out.

@cdOut Thanks for taking care of the remaining checkboxes. This https://github.com/Expensify/App/pull/43251#issuecomment-2176992855 is the last issue I was able to find by testing the Details Revamp Doc.
♻ I will start working on the checklist now.

@ikevin127 I've added the test steps for tasks, make sure to compare the options available in the three dot menu with the ones displayed in RHP. I've also included a demo video in the web section of videos if you want to check that also.
I'm currently investingating the issue you've pointed out with the Hold PromotedAction not displaying for your specific case.

@cdOut Thanks! I started on the checklist and found a discrepancy in one of the testing steps:
Workspace Rooms & Groups:

The PromotedActionsBar should be showing the Pin and Share actions, Join if the user is a guest.

As a guest user of the room -> I'm not seeing the Join button in the PromotedActionsBar but in the room header instead:

MacOS: Safari
https://github.com/Expensify/App/assets/56457735/bb18122c-7a99-4357-a93c-c449e3ccbd26

I think this is expected, but the testing step should be corrected in order to not confuse the testers 😅

Reviewer Checklist

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

[x] I verified the steps for local testing are in the Tests section
[x] I verified the steps for Staging and/or Production testing are in the QA steps section
[x] 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)
[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)

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

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

[x] If there are any errors in the console that are unrelated to this PR, I either fixed them (preferred) or linked to where I reported them in Slack
[x] I verified proper code patterns were followed (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 the left part of a conditional rendering a React component is a boolean and NOT a string, e.g. myBool && <MyComponent />.
[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 is localized by adding it to src/languages/* files and using the translation method
[x] I verified all numbers, amounts, dates and phone numbers shown in the product are using the localization methods
[x] I verified any copy / text that was added to the app is grammatically correct in English. It adheres to proper capitalization guidelines (note: only the first word of header/labels should be capitalized), and is either coming verbatim from figma or has been approved by marketing (in order to get marketing approval, ask the Bug Zero team member to add the Waiting for copy label to the issue)
[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 verified that this PR follows the guidelines as stated in the Review Guidelines
[x] 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)
[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] 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 any new file was added I verified that:

[x] 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

[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(theme.componentBG)

[x] 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.
[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] If the PR modifies a component or page that can be accessed by a direct deeplink, I verified that the code functions as expected when the deeplink is used - from a logged in and logged out account.
[x] If the PR modifies the UI (e.g. new buttons, new UI components, changing the padding/spacing/sizing, moving components, etc) or modifies the form input styles:

[x] I verified that all the inputs inside a form are aligned with each other.
[x] I added Design label and/or tagged @Expensify/design so the design team can review the changes.

[x] 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.
[x] 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.
[x] I have checked off every checkbox in the PR reviewer checklist, including those that don't apply to this PR.

Screenshots/Videos

Android: Native
Workspace Rooms & Groups
https://github.com/Expensify/App/assets/56457735/98665865-7070-40ec-938d-ff6b49609bf4
IOU / Expense Reports
https://github.com/Expensify/App/assets/56457735/1bad11f7-4784-4c4f-a794-56242f8624ad
Tasks
https://github.com/Expensify/App/assets/56457735/11bd6377-772b-4beb-9a17-d71b70d1fdb2

Android: mWeb Chrome
Workspace Rooms & Groups
https://github.com/Expensify/App/assets/56457735/68049e04-67a3-46e7-aa71-0b7ba38bd8d4
IOU / Expense Reports
https://github.com/Expensify/App/assets/56457735/4be90329-0703-4fec-80e3-b4f96a0b7fb6
Tasks
https://github.com/Expensify/App/assets/56457735/558420d8-2fea-44b2-ad23-688168b96d1b

iOS: Native
Workspace Rooms & Groups
https://github.com/Expensify/App/assets/56457735/0ac49d6c-b6aa-4441-9574-09734fb64c97
IOU / Expense Reports
https://github.com/Expensify/App/assets/56457735/8b97f9cf-3ca4-469e-9300-358c83c4caf1
Tasks
https://github.com/Expensify/App/assets/56457735/6db763ea-30f8-4721-85b5-22613e156645

iOS: mWeb Safari
Workspace Rooms & Groups
https://github.com/Expensify/App/assets/56457735/98cb57eb-7b2b-4f7a-add3-0be109efcf48
IOU / Expense Reports
https://github.com/Expensify/App/assets/56457735/cd4262ab-4bbb-431d-9e5d-5feb6708b775
Tasks
https://github.com/Expensify/App/assets/56457735/65ea7639-dee1-4788-87fb-62f22b5c6be6

MacOS: Chrome / Safari
Workspace Rooms & Groups
https://github.com/Expensify/App/assets/56457735/99f5fe72-77fc-4406-af3c-76bc359de5ad
IOU / Expense Reports
https://github.com/Expensify/App/assets/56457735/902a662b-4b4b-4cce-aa05-e9f10c0c20e2
Tasks
https://github.com/Expensify/App/assets/56457735/f674b24f-df2f-4d21-8759-f38fb0829e99

MacOS: Desktop (empty)

Note: Not able to test / open RHP from header on Desktop since there's currently a deploy blocker (not related to this PR) which I mentioned in https://github.com/Expensify/App/pull/43251#issuecomment-2177138554.

@cdOut The only discrepancy that I found between platforms while testing is that on Android / iOS Native in the Room RHP -> the in [Workspace Name] subtitle doesn't show-up at all (most likely native styling issue, see below).
Once this issue and this previous one are addressed -> I will double-check then ✅ Approve if everything looks good.

iOS: Native
iOS: mWeb

Android: Native
Android: mWeb

In this screenshot here:

I don't think I would expect that tapping directly on the workspace name would navigate you to the workspace settings. I think tapping anywhere on this row should just take you to the push page for editing the room name. Is that your understanding too @Expensify/design?

I don't think I would expect that tapping directly on the workspace name would navigate you to the workspace settings. I think tapping anywhere on this row should just take you to the push page for editing the room name. Is that your understanding too @Expensify/design?

Yep!

I don't think I would expect that tapping directly on the workspace name would navigate you to the workspace settings. I think tapping anywhere on this row should just take you to the push page for editing the room name. Is that your understanding too @Expensify/design?

Yes that's my expectation as well.

@ikevin127 I've fixed all the listed issues and provided updated test videos / steps for people who don't have access to the design doc. This should be ready to be retested, please take special care to check whether the delete button works properly for all available cases, since this is the flow that we had to revamp the most for this PR from the previous implementation and it might cause problems in some very specific cases.

I've added the CONST suggestion and merged with newest main, there was a problem with action.originalMessage getting deprecated so I swapped it with the updated ReportActionsUtils.getOriginalMessage().
You could retest whether after the expense deletion you get redirected to the correct page, I've tested it myself and it should be good. After that it should be ready to merge.

Triggering a test build to QA

Tests well! Looks great. I'm going to go ahead and :shipit:

: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 https://github.com/grgia in version: 9.0.1-0 🚀

platform
result

🤖 android 🤖
success ✅

🖥 desktop 🖥
success ✅

🍎 iOS 🍎
success ✅

🕸 web 🕸
success ✅

@grgia @ikevin127 It would be great if you could help with the following about this PR.

Difference between member and invitee/guest.
How to invite as a guest to workspace chat , user created workspace room and to IOU expense? ( By mentioning the user?)

🚀 Deployed to production by https://github.com/yuwenmemon in version: 9.0.1-19 🚀

platform
result

🤖 android 🤖
success ✅

🖥 desktop 🖥
success ✅

🍎 iOS 🍎
success ✅

🕸 web 🕸
success ✅


chore: improve dev workflow

Includes #101 and #102.

@satya164 Other than the trailing comma issue in rn-cli.config.js, everything is working fine 👍

Damn, the eslint-disable didn't work for some reason. Fixed.


[move] Remove into_module from disassembler, bytecode-source-map, and move-bytecode-viewer

Motivation
The method CompiledScript::into_module is deprecated, and ideally it ought to be deleted. This set of commits aims to remove it from the bytecode-source-map module without otherwise changing the behavior of the compiler.
Have you read the Contributing Guidelines on pull requests?
Yes.
Test Plan
The existing suite of tests.
Related PRs
This work is branched off of commits made as part of #8536; specifically, the commit named [move] Bounds checker runs on script or module, which adds methods to BinaryIndexedView that are used in this pull request.

Still a WIP because I'm still working through files in the module, like mapping.rs.

@tnowacki you mentioned during our into_module breakout chat on June 10 that this part of the codebase might be tricky and to ping you when attempting it. This is my first pass, let me know what you think!
One big change here is that BinaryIndexedView only keeps a reference to its underlying module or script, so the disassembler and source maps no longer own the module or script data they represent, and so I introduced lifetime markers to ensure that the underlying resource isn't freed prematurely.

/land


Log levels broken

What happened
It seems that since fe3dba90d8cc6f947846e6a099a2562e7c2f88b1, --log DEBUG has no effect when running kube-hunter.
Steps to reproduce:

Checkout repository
docker run -it $(docker build . | tail -n 1 | cut -d" " -f3) --log DEBUG --pod

Reverting to fd4d79a8534fe3967162c18770a208871d8403c9 restores proper logging behavior.
Expected behavior
Debug messages should show up when --log DEBUG is specified.

Hi @invidian !
Thanks for reporting!
This is surely a regression since #283.
The bug origins in a call to logging.basicConfig being done before the configuration set.
We should avoid using logging default logger and use a custom one to prevent unexpected behavior.


Access Violation crash after several scans.

Im using this project in my app to scan qr codes but after several scans the application exits with an access violation error (no exception, just an exit code). Does anyone else have this problem?

I think I've seen this - haven't had opportunity to debug it to the point of understanding quite where it's coming from yet though I'm afraid.

Yea I am coming across it as well.  I even tried changing the result of the callback to just a string with no luck.  It always happens on the second result, no matter what for me.  Is it maybe due to the time out await?  I made the time out only 5 seconds and removed everything from memory, but that didnt work.  There has to be something that is getting called in memory where it doesnt exist.

The thread 0x414 has exited with code 0 (0x0). The thread 0xc60 has exited with code 0 (0x0). The thread 0x1338 has exited with code 0 (0x0). Exception thrown: 'System.Exception' in Assembly-CSharp.dll The program '[2232] Hololens.exe' has exited with code -1073741819 (0xc0000005) 'Access violation'.

I reworked the code to try and fix this - the solution is perhaps a bit less 'elegant' than it was but I can run many times and I don't see this crash any more.

Unfortunately, it doesn't recognize any codes anymore for me.

Woops - that doesn't sound good. Is this in Unity? in Xaml? In Both?

We are using the scanner on the hololens, which is a unity project.

That's not good. I just cloned down the latest bits. I opened the Unity project. I built the Visual Studio solution. I deployed from Visual Studio to my HoloLens. I scanned the same QR code 10 times one after the other using what I deployed. The QR code was on my 2nd monitor approximately 40-50cm in front of the HoloLens and it's approximately 3cm high and was generated from the QR code generator that you find if you search Bing for QR codes.
So, this seems to be working for me - can you provide more detail or maybe help debug what's happening on your system?

I found the problem. It was on my side. Previously, a team member of ours had modified the code to read barcodes instead. Of course, this was gone once I recompiled the code with the latest version of MediaFrameQrProcessing. I wasn't aware that a modification had been necessary for the code to recognize barcodes. Sorry for the confusion. Thank you for fixing the access violation :-)

No problem - glad that it's been improved by the fix.


Replaces old eye damage effect

About the PR
As per discussion #10717, the effect is be replaced with a dim effect that scales with the damage.
Closes #10717
Obsoletes #10727
Screenshots

Changelog

:cl:

tweak: The blurry effect from eye damage has been replaced with a dim overlay filter.

Screenshots pls

normal

low damage

high damage

Why not do a normal blur instead of the vertical blur Rane made? Or does that cause headaches as well?
Or, combine it with this shader.

Why not do a normal blur instead of the vertical blur Rane made? Or does that cause headaches as well? Or, combine it with this shader.

Im not inherently opposed to a blur, but the issue is that this effect is permanent, meaning that it can be active for minutes at a time. For something like the drunk shader, its fine, because you opt into it activating and it only persists for a short period of time.
This effect, on the other hand, is a negative effect for using a common tool, which is permanent unless you can acquire treatment.

The drunk shader persists for a really long time

The drunk shader persists for a really long time

"Really long time" based on how much of a substance you voluntarily drink is different from a permanent effect.

The drunk shader persists for a really long time

long time > forever

I agree the old shader sucks and a regular blur would be better, but I don't see why you couldn't just use a welding mask to avoid the eye damage in the first place.

Eye damage is easier to prevent / cure / mitigate than the other blurry vision shader yeah

"Really long time" based on how much of a substance you voluntarily drink is different from a permanent effect.

 There are multiple non-alcoholic sources of it too

Eye damage is easier to prevent / cure / mitigate than the other blurry vision shader yeah

How is getting specific chems easier than just waiting?

It's a tradeoff then, you either weld without protection and get lasting eye damage or you just don't weld.

How is getting specific chems easier than just waiting? Also prevention isnt always an option in high stress/low supply situations, wherein you'll be stuck with the annoying effect.

Glasses instantly correct it, you can get oculine from botany too in addition to medbay, and oculine instantly corrects it. The other blur effect you are forced to wait out without any method to shorten it, and it can persist for like 15 minutes

I don't know how to formulate my point other than this:
I don't want my eyes to physically hurt because I did an action in game and I might not know/be able to prevent/cure/mitigate it.

I don't know how to formulate my point other than this: I don't want my eyes to physically hurt because I did an action in game and I might not know/be able to prevent/cure/mitigate it.

Feel free to add a different method of blur, the fact people don't complain about the drunk shader despite constantly getting involuntarily stuck with that too means it might just be that particular implementation. This shader just looks like sunglasses

How often are people getting involuntarily stuck with it? I've personally never had the blur shader unless from alcohol, which never lasted longer than a minute or so.

How often are people getting involuntarily stuck with it? I've personally never had the blur shader unless from alcohol, which never lasted longer than a minute or so.

There are several sources of it besides alcohol, I get it basically whenever someone gives me bicaridine

heres my suggestion. aaahhh owww my retinas are burned

You know, this discussion is not particularly constructive, we should be discussing ideas for a better effect instead.
I propose a different kind of blur that doesn't hurt people's eyes.  I propose a different kind of blur that doesn't hurt people's eyes. Or maybe fuck up the screen like Emisse just posted

Drunkenness shows it may have just been the particular sort of blur used, I agree with using a different kind of blur

I think that considering that this shader is permanent until action has been taken to remove it, it's best that it remain somewhat mild. Let's not forget that eye damage has several other effects as well. I would say that this one isn't any more disruptive than the previous one.

I don't really see a huge difference between a shader that has lasted me 3/4 of a decently long round before (dizziness shader) and a shader that's permanent. It might matter more on places with like 6 hour long rounds

I think the effect in this PR is way too mild. The effect should still encourage people to take precautions before welding (or not weld at all, if you can't get your hands on welding masks!) and seek treatment when you end up with it. We should only ensure that it doesn't hurt people's eyes. If the effect is an inconvenience, then it's doing it's job.

I also feel like a basic gaussian shader would be good. The current blur is pretty straining to the eyes.

How about something like this example

How about something like this example
This would be the last stage of it though. Obviously make it fade in from the sides as it gets worse

i quite like this option. Unfortunately, my shader knowledge is quite limited and i wouldn't really have an idea of how to make this. Considering that #10727 is being offered as an alternative solution to this one, i think having this as an intermediate solution while a better one has not been created would be a decent compromise.

Not really sure what part of that video is intended or is just 240p.

I agree the old shader sucks and a regular blur would be better, but I don't see why you couldn't just use a welding mask to avoid the eye damage in the first place.

The issues were mostly encountered on Outer Rim, where at the time it wasn't actually possible to obtain and making Oculine could take hours.
This shader actively causes eye strain and must be removed.


[feat] - replace std library regex engine with go-re2

Description:
This PR replaces the standard library regex with go-re2 in all instances except for newLineRegex. The standard library regex engine is more performant than go-re2 for small regex patterns.
Profile

Benchmarks

git log -p output for this repo. [https://github.com/gitleaks/gitleaks]

Before ~58s:

After ~9s:

gitlab repo [https://gitlab.com/gitlab-org/gitlab]

Before ~49s:

After ~14s:

linux repo [https://github.com/torvalds/linux]

Before ~1m52s:

After ~1m:

Checklist:

[ ] Does your PR pass tests?
[ ] Have you written new tests for your changes?
[ ] Have you lint your code locally prior to submission?

Is there a memory profile available? When TruffleHog switched, a few people complained about the large jump in memory usage.

Is there a memory profile available? When TruffleHog switched, a few people complained about the large jump in memory usage.

I shared one with Zach but can add it here as well. I agree—the cost is noticeable, and the tradeoff might not be worth it. From what I understand, the WASM runtime module has an initial cost of 4MB, which increases with each rule, though the per-rule growth is significantly smaller.
I mentioned this to Zach, but feel free to close if the tradeoff isn't worth it.

It's a shame that Go doesn't have #if directives. Then you could easily toggle between both.

christ almighty these numbers are impressive

It's a shame that Go doesn't have #if directives. Then you could easily toggle between both.

I was reading some C code earlier and thought the exact same thing... 😢

letterip


Add missing docstrings in api.py

Added docstrings missed in #74.

Hrm, lots of commits here, but the changes look fine.

@stefanv fixed the commit history.


Set compiler flag 'unwind_info' to false if specified in config

Regarding issue #6541, it was reported that unloading a large module on macOS can result in significantly long loading times. One suggestion was to disable unwind frames, but this didn't seem to have any effect. Upon further investigation, it appears that this issue may be a regression introduced in a commit made last year in August. The specific commit can be found here: link to commit.
Currently, there is only support for enabling the compiler flag, and there is no way to disable it. This means that the config flag is enabled by default, and there is no option to turn it off. The suggested pull request addresses this issue by checking if native_unwind_info is disabled and inserting the flag into the compiler config if necessary.
I would like to have it confirmed if this means that all crane lift users have been forced to compile with unwind frames, always, or perhaps I am missing something.
Also second question, disabling this flag, it would only affect GC tracing and debugging right? So should be fine to expose this to users if they use neither of those?
As temporary work around i can do:
unsafe {
    config.native_unwind_info(false);
    config.cranelift_flag_set("unwind_info", "false");
}

I think it might be important to make the native_unwind_info field be an Option<bool> or similar, so that if nobody calls Config::native_unwind_info then we know we can set it whichever way makes sense.
Also, I think the existing error messages, about unwind info being necessary for profiling, might be misleading. On Linux, perf defaults to frame-pointer unwinding, which I believe is sufficient today on x86 since we always produce frame pointers. We also have a Cranelift setting to force frame pointers, which we could use when profiling instead of registering unwind info. I think Nick said we'll need more sophisticated unwinding once we have exception-handling support though.
In short, I think we need to sort out exactly when native unwind info is actually necessary before reviewing this PR in detail, because I think the overall logic may not be right.

I think it makes sense to discuss when native unwind info is necessary separately from #6541, which is about native unwind info being slow. But I don't think we need to open a new issue for it; this PR is a fine place to sort out the details, in my opinion.
I'm hoping both @bjorn3 and @fitzgen can give more context on what the native unwind info is actually used for.

I tested locally, and we do indeed produce identical binaries, both with .eh_frame (I'm on Linux) regardless whether this option is set to true or false. Whoops! We should definitely fix this!

Also, I think the existing error messages, about unwind info being necessary for profiling, might be misleading. On Linux, perf defaults to frame-pointer unwinding, which I believe is sufficient today on x86 since we always produce frame pointers. We also have a Cranelift setting to force frame pointers, which we could use when profiling instead of registering unwind info.

A couple small corrections / refinements:

perf defaults to frame pointers but doesn't always use them. In fact, there are libc asm functions that often end up in Wasmtime profiles that don't preserve frame pointers IIRC but do have CFI pragmas, and perf will only capture these correctly when you're using DWARF stack walking. (I may have this inverted, though). In any case, assuming that perf is always in frame pointers mode is a no go.

Wasmtime always forces the compiler to emit frame pointers (to enable fast+simple stack walking) regardless of platform. (The Cranelift x86 backend will always emit frame pointers regardless of the Cranelift setting to preserve them or not.)

So with (1), the message saying native unwind info is required for profiling is probably a bit strong, but it also isn't not required. It is subtle, and emitting the unwind info in that case, even if it doesn't end up used by the profiler's exact configuration, doesn't hurt so I'd like to continue to emit native unwind info whenever profiling support is configured.

Backing up a bit: the reason we enable native unwind info by default is to be good citizens of the platform and do our best to interoperate with external tooling (such as profilers or debuggers or whatever) that are usually expecting to see the platform's unwind info. We don't rely on it for correctness, which is why we can (modulo bugs) turn it off. I think the good-citizen angle is still well-motivated, so I don't want to change defaults here.

Option<bool> and then reading it something like self.native_unwind_info.unwrap_or(DEFAULT_NATIVE_UNWIND_INFO) makes sense to me.

I might have missed some context reading over this, but how come the profiling strategy is consulted when determining whether to enable this? I know that jitdump/perfmap/guest don't use native unwinding information (perf can't find it even if we emit it), although I don't know about vtune. Given that I don't think we should force enable the unwinding information if profiling is enabled, instead sticking to whatever the default was.

I might have missed some context reading over this, but how come the profiling strategy is consulted when determining whether to enable this? I know that jitdump/perfmap/guest don't use native unwinding information (perf can't find it even if we emit it), although I don't know about vtune. Given that I don't think we should force enable the unwinding information if profiling is enabled, instead sticking to whatever the default was.

Ah, I must be operating on outdated information (just like the configuration code itself must have been) because I was under the impression that perf used this information when configured to consume DWARF.
If not, then yeah, we can probably decouple profiling and native unwind info.

Hey @fitzgen, the description of Cranelift's unwind_info setting says:

This increases metadata size and compile time, but allows for the debugger to trace frames, is needed for GC tracing that relies on libunwind (such as in Wasmtime), and is unconditionally needed on certain platforms (such as Windows) that must always be able to unwind.

Does Wasmtime GC still require libunwind? I assumed the fast stack walking is used there now.
I'm wondering if we should make this Cranelift setting default to "false" at some point.

I'm wondering if we should make this Cranelift setting default to "false" at some point.

That would break panic backtraces. The only way wasmtime can influence panic backtrace printing is by registering unwindinfo. It can't hook unwinding.

I mean in wasmtime. And there not the unwinding, but the unwinding to get a backtrace as printed to the user. Backtrace printing is entirely unaffected by catch_unwind.

Does Wasmtime GC still require libunwind? I assumed the fast stack walking is used there now.

That is correct, we no longer require libunwind since the fast stack walking patches. Those docs are just outdated.

CI says that this PR breaks fuzzing because crates/fuzzing/src/generators/config.rs may randomly try to disable unwind info even on Windows. So we need the config generator to not do that on Windows.
I think these configs are never used for cross-compiling, so it should be good enough to use cfg!(target_os = "windows") rather than having to plumb the target through. But I'd appreciate if @fitzgen or @alexcrichton could check me on that.
Assuming that's true, then the to_wasmtime method in crates/fuzzing/src/generators/config.rs can just replace
.native_unwind_info(self.wasmtime.native_unwind_info)

with
.native_unwind_info(cfg!(target_os = "windows") || self.wasmtime.native_unwind_info)

@TimonPost would love to see this get merged, is there anything I can do to help?

Good one, i'll patch it up, should be ready in a bit!

Rebased and updated the PR. @fitzgen can you review this?

I'm not sure about the CI failures, but just figured I'd popin to say I'm using this patch with #6896 and it fixed some issues with C++ exceptions and wasmtime used in conjunction (there is a lock in libgcc that was having some contention issues).


(General/JSON) Sidebar sheet selection doesn't work properly

Hi,
I noticed something odd recently. Everytime I deselect one of my sheets in the list and select another sheet instead, the exporter does not export the newly selected sheet. It only does that when i export the sheet again. I am guessing this has something to do with the sidebar not saving settings properly?

Sorry for the delayed response. It sounds like event listeners are not getting reset properly after reloading the sidebar. I'll take a look and see if I can fix the issue. A quick workaround should be setting the export target to current sheet only, then resetting it to custom and selecting your sheets.

No problem, yes i noticed that too. Apparently selecting "All sheets" resets the whole thing as well.

I get this bug too. Apparently, it happens only if you have more than 12 sheets in the document or so. After some time, it stops responding to tick on/off. And I noticed it's always the last sheet which stops working. Every time I add a new sheet, I dread of ticking it, cuz it's a nightmare :(

I get this bug too. Apparently, it happens only if you have more than 12 sheets in the document or so. After some time, it stops responding to tick on/off. And I noticed it's always the last sheet which stops working. Every time I add a new sheet, I dread of ticking it, cuz it's a nightmare :(

Hmm. I'll take a look at the issue again this weekend and see if I can replicate the issue. Would you be able to share a sample document that is affected by the issue so I know I am seeing the same thing as you @killeriukas ?

I get this bug too. Apparently, it happens only if you have more than 12 sheets in the document or so. After some time, it stops responding to tick on/off. And I noticed it's always the last sheet which stops working. Every time I add a new sheet, I dread of ticking it, cuz it's a nightmare :(

Hmm. I'll take a look at the issue again this weekend and see if I can replicate the issue. Would you be able to share a sample document that is affected by the issue so I know I am seeing the same thing as you @killeriukas ?

Here is a link to the document: https://docs.google.com/spreadsheets/d/1lmBM23NO2nvRxDqcwm9S0PtaMBYEytqWyUzexMNEqV0/edit?usp=sharing
What I do to get the bug easily is: select to export it as a custom - then select all, then scroll to the very bottom of the list and start unselecting the last ones, and you will notice that the ALL flag does not untick. It seems that the last X number of sheets do not reflect on if they have been selected or not. This is horrible for me, cuz every time I add a new table it doesn't export the selection properly :(

I've found the problem affecting you specifically @killeriukas. The sheet called "Paul's Notes" is what breaks the toggles. If you move it to be the last tab, all sheets before it can be checked and unchecked as expected.
I'm assuming that this is because of the apostrophe in the sheet name, which makes sense with how those toggles are generated and listened to. I think the fix there is to convert special characters in sheet names to their encoded versions. So Paul's Notes would become Paul's Notes on the backend (but would still export as just Paul's Notes). For now, getting rid of the apostrophe in the sheet name should fix your problems.

I've found the problem affecting you specifically @killeriukas. The sheet called "Paul's Notes" is what breaks the toggles. If you move it to be the last tab, all sheets before it can be checked and unchecked as expected.
I'm assuming that this is because of the apostrophe in the sheet name, which makes sense with how those toggles are generated and listened to. I think the fix there is to convert special characters in sheet names to their encoded versions. So Paul's Notes would become Paul's Notes on the backend (but would still export as just Paul's Notes). For now, getting rid of the apostrophe in the sheet name should fix your problems.

OMG - yes. The ' breaks up the string, which in turn breaks the listeners. Darn. Thank you very much for finding what the problem was. I just removed the ' and the problem went away. Xaxaxa.
It shouldn't be hard to fix this to future proof the plugin so that no one else would bother you with the same problem down the line. I very much appreciate your help! Thank you.

Just pushed the v49 update which should fix this issue.

Annnnnd it's still there!

I'm not sure if this is the same problem, but for me it only seems to export the last sheet in my doc. I tried doing it with all defaults, and selecting specific sheets through the sidebar, and other things. But no matter what I do it just exports my last sheet. I can't seem to get it to export any of the other sheets. Actually, yeah, I just tried re-ordering the sheets. It's always just exporting the last one!

Actually...it's like completely random as to what it decides to export.

Hmm, interesting. You should be able to toggle the "All" checkbox on and off then select your desired sheets. That usually seems to fix the issue. I'm still hunting down the cause, but I'm hoping to find it by the week's end.

I've tried a whole bunch of manipulations, incl opening and closing the doc, reinstalling, etc, and they all seem to change the behavior, but I haven't got it to export anything like the whole doc yet  (and it's not that large a doc, like 4 sheets and a smallish amount of data in each). Some possibly useful factoids: It's not my doc (shared with me) and there's another user on it.  I'll trying copying it into my drive (which will make it unshared, so no multiple users) and see if that helps.

Multiple users adjusting options for the same sheet at the same time have caused problems before, it's one of the usability problems on the roadmap to try and fix. It shouldn't be the sheet array option since that just exports sheets as JSON arrays instead of JSON objects. What other options do you have enabled?


The parse exception clues are off sometimes

I'm using jju to provide more helpful errors when parsing invalid JSON files. I noticed that sometimes the clues about the location of the error (line:column) are off and it seems it mostly adds 1 to either the line or the column.
Here's an example of the column index being off:
var jju = require('jju');
jju.parse('{"a":1,\n"b":{"ba":"foo"\n}', {mode: 'json'});

When I run it I get:
Unexpected end of input at 3:3
}
 ^

The end of input is at 3:2 and not 3:3. Am I missing something?

When the error is occured, current caret position is at 3:3. So parser expects something at 3:3, but can't find anything.
I don't think this one is misleading, an end of input is an end of input. Are there any other similar cases?

Why is the caret at 3:3? The string ends with '}' which is located at 3:1 so I expect the end of input to be at 3:2. Also if you look at the output the ^ is pointing to the next character after '}' so that also seems to suggest the location should be 3:2.
Here's another case that I find a bit confusing, when you parse this:
{
    "a": "b",
    "c": 5,
}

You get:
Trailing comma in object at 4:1
}
^

The trailing comma is at the end of the third line and not at the beginning of the fourth line. In this case I understand the index, since 4:1 is the point where the parse actually failed. That said, I find it confusing that we say the issue is a "Trailing comma in object at", but instead of telling the user where that comma is located, we give the index of the closing curly brace. If you add some extra new lines before the closing brace the index would change accordingly, even due the actual problem, the trailing comma, is at the same location.


packages starting with "@" are not working in allowlist

With the following allowlist in the config, any packages starting with the "@" are still being identified as vulnerabilities in the report unless the full path is specified.
{
  "$schema": "https://github.com/IBM/audit-ci/raw/main/docs/schema.json",
  "critical": true,
  "skip-dev": true,
  "allowlist": [
    // these do not work
    "@optimizely/react-sdk",
    "@storybook/react",
    "*|@storybook/addon-actions>*",
    "@svgr/webpack",

    // the full path works
   "GHSA-g4rg-993r-mgx7|@storybook/react>@storybook/core>react-dev-utils>shell-quote",

    // standard module works
    "minimist"
  ]
}

Run with config file
npx audit-ci --config audit-ci.jsonc

Here are the results, which I have elided for clarity
npx audit-ci --config audit-ci.jsonc
npx: installed 39 in 2.385s
audit-ci version: 6.6.1
Yarn audit report results:
{
...
Modules to allowlist: minimist, @optimizely/react-sdk, @storybook/react, @svgr/webpack.
Found vulnerable allowlisted modules: minimist.
Consider not allowlisting modules: @optimizely/react-sdk, @storybook/react, @svgr/webpack.
Found vulnerable advisory paths:
GHSA-33f9-j839-rf8h|@storybook/react>@storybook/core>@storybook/ui>@storybook/components>immer
GHSA-33f9-j839-rf8h|@storybook/react>react-dev-utils>immer
...
GHSA-phwq-j96m-2c2q|@storybook/react>@storybook/core>ejs
GHSA-76p3-8jx3-jpfq|@storybook/react>@storybook/core>postcss-loader>loader-utils
GHSA-g4rg-993r-mgx7|@storybook/react>react-dev-utils>shell-quote
..
GHSA-896r-f27r-55mw|@optimizely/react-sdk>@optimizely/optimizely-sdk>json-schema
...
GHSA-76p3-8jx3-jpfq|@svgr/webpack>loader-utils
GHSA-hgjh-723h-mx2j|@storybook/react>react-dev-utils>sockjs-client>url-parse
...
GHSA-6h5x-7c5m-7cr7|@storybook/react>react-dev-utils>sockjs-client>eventsource
...
Failed security audit due to critical vulnerabilities.
Vulnerable advisories are:
https://github.com/advisories/GHSA-33f9-j839-rf8h
https://github.com/advisories/GHSA-phwq-j96m-2c2q
https://github.com/advisories/GHSA-76p3-8jx3-jpfq
https://github.com/advisories/GHSA-g4rg-993r-mgx7
https://github.com/advisories/GHSA-896r-f27r-55mw
https://github.com/advisories/GHSA-hgjh-723h-mx2j
https://github.com/advisories/GHSA-6h5x-7c5m-7cr7
Exiting...

Hi there, just trying to confirm the implementation here based on the logs that you've shown:

@optimizely/react-sdk: if I recall the implementation correctly, allowlisting modules only allowlists direct advisories of the module itself. It will not work with transitive advisories. In this case, none of the advisories are @optimizely/react-sdk itself. Rather, it's things like json-schema. If you want all transitives for @optimizely/react-sdk, you'd do @optimizely/react-sdk>* (I think that's how it works, it's late, forgetting off the top of my head :))
@storybook/react: same
*|@storybook/addon-actions>* none of these have the first level of advisory being @storybook/addon-actions, so it won't allowlist.
@svgr/webpack same as first

Thanks for looking into this. I changed the config per your comment and they now all get allow listed. Thanks!
  "allowlist": [
  "*|@optimizely/react-sdk>*",
  "*|@storybook/react>*",
  "*|@storybook/addon-actions>*",
  "*|@svgr/webpack>*",
  "minimist"
 ]


fix last msg

Codecov Report

Merging #83 into master will decrease coverage by 0.29%.
The diff coverage is 0%.

@@            Coverage Diff            @@
##           master      #83     +/-   ##
=========================================
- Coverage   59.75%   59.45%   -0.3%
=========================================
  Files           9        9
  Lines        1210     1216      +6
=========================================
  Hits          723      723
- Misses        400      406      +6
  Partials       87       87

Impacted Files
Coverage Δ

lib/msgapi/squish.go
71.95% <0%> (-0.54%)
:arrow_down:

lib/msgapi/msg.go
65.27% <0%> (-0.92%)
:arrow_down:

lib/msgapi/jam.go
59.87% <0%> (-0.38%)
:arrow_down:

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 f923743...1329efc. Read the comment docs.


Crash when clicking "your trip" in Ungap The Map

Full stacktrace in: https://gist.github.com/XioNoX/7e2cf2d27f83247c645a53062a0bd378

Thanks for reporting! Something broke while rendering a route:
  17:     0x55881062ab68 - geom::polyline::PolyLine::to_thick_ring::he7c76c6ed364015a
  18:     0x55881062ae0a - geom::polyline::PolyLine::to_thick_boundary::h0cfa22e9a1c6dcd3
  19:     0x55880faf065e - game::ungap::trip::results::RouteDetails::new_route::h7bad7974c2f8a51c

To reproduce more easily, it'd be good to know the start/endpoint of the trip you made. By default this tab will load the previous trip you made, which is stored in data/player/routes/fr/brest/city.json. Assuming there's nothing private about the routes you've made there, could you share the endpoints for the one that seems to be crashing?

Unfortunately I don't have it anymore but I'll try to reproduce it.


Mixed Model and Contrast

Hello Lukas,
Thank-you for Diffcyt.
I had a quick question about setting up contrast matrix for mixed model. I hypothesize that the cell cluster proportions depend on the condition (case-status, binary) when corrected for age and sex (fixed effect), and the date of batch processing (categorical, 4 values, random effect)
ei <- metadata(sce)$experiment_info
> head(ei)
  sample_id condition    sex age        date n_cells
1       E23   Control Female  46 16-AUG-2019  150000
2       E25   Control   Male  42 28-AUG-2019  150000
3       E26   Control Female  45 16-AUG-2019  150000
4       E28   Control Female  52 16-AUG-2019  150000
5       E29      Case Female  40 28-AUG-2019  150000
6       E30      Case Female  42 28-AUG-2019  150000
> str(ei)
'data.frame':	102 obs. of  6 variables:
 $ sample_id: Factor w/ 102 levels "E23","E25","E26",..: 1 2 3 4 5 6 7 8 9 10 ...
 $ condition: Factor w/ 2 levels "Control","Case": 1 1 1 1 2 2 2 1 1 2 ...
 $ sex      : Factor w/ 2 levels "Female","Male": 1 2 1 1 1 1 1 1 1 1 ...
 $ age      : num  46 42 45 52 40 42 52 52 46 62 ...
 $ date     : Factor w/ 4 levels "08-OCT-2019",..: 2 4 2 2 4 4 1 2 2 4 ...
 $ n_cells  : num<PHONE_NUMBER>00<PHONE_NUMBER>00<PHONE_NUMBER>00<PHONE_NUMBER>00<PHONE_NUMBER>00 ...
da_formula1 <- createFormula(ei,
                             cols_fixed = c("condition", "age", "sex"),
                             cols_random = "date"
)
> da_formula1[["formula"]]
y ~ condition + age + sex + (1 | date)
contrast <- createContrast(c(0, 1, 0, 0))
> contrast
     [,1]
[1,]    0
[2,]    1
[3,]    0
[4,]    0

da_res1 <- diffcyt(sce,
                   formula = da_formula1, contrast = contrast,
                   analysis_type = "DA", method_DA = "diffcyt-DA-GLMM",
                   clustering_to_use = "meta10", verbose = T
)
topTable(da_res1, format_vals = T)
> topTable(da_res1, format_vals = T)
DataFrame with 10 rows and 3 columns
   cluster_id     p_val     p_adj
     <factor> <numeric> <numeric>
1          1   0.00e+00  0.00e+00
2          2   0.00e+00  0.00e+00
3          3   0.00e+00  0.00e+00
4          4   0.00e+00  0.00e+00
5          5   0.00e+00  0.00e+00
6          6   0.00e+00  0.00e+00
7          7   0.00e+00  0.00e+00
8          8   0.00e+00  0.00e+00
10         10  1.78e-15  1.97e-15
9          9   7.95e-01  7.95e-01

Either I am about to publish a Science / Nature paper or I messed up (the probability of latter is close to 1).
Could you please let me know where am I messing up?
Also, like Toptable, is there a quick & easy way to check the effect size?
Thanks a lot!
Vivek

Just an update, I tried running the same using testDA_GLMM(), which requires using calcCounts(), which needs running generateClusters(). However,
> sce <-  generateClusters(sce)
Error: Argument 'exprs' must be numeric matrix with colnames attribute set
> typeof(sce@assays@data@listData[["exprs"]])
[1] "double"
> dim(sce@assays@data@listData[["exprs"]])
[1]       14 15257202
> sce@assays@data@listData[["exprs"]] <- as.matrix(sce@assays@data@listData[["exprs"]])
> typeof(sce@assays@data@listData[["exprs"]])
[1] "double"
> dim(sce@assays@data@listData[["exprs"]])
[1]       14 15257202
> sce <-  generateClusters(sce)
Error: Argument 'exprs' must be numeric matrix with colnames attribute set

Tried with normalize = T:
> da_res1 <- diffcyt(sce,
+                    formula = da_formula1, contrast = contrast,
+                    analysis_type = "DA", method_DA = "diffcyt-DA-GLMM",
+                    normalize = T, norm_factors = "TMM",
+                    clustering_to_use = "meta20", verbose = T
+ )
using SingleCellExperiment object from CATALYST as input
using cluster IDs from clustering stored in column 'meta20' of 'cluster_codes' data frame in 'metadata' of SingleCellExperiment object from CATALYST
calculating features...
calculating DA tests using method 'diffcyt-DA-GLMM'...
There were 20 warnings (use warnings() to see them)
> topTable(da_res1, format_vals = T)
DataFrame with 20 rows and 3 columns
    cluster_id     p_val     p_adj
      <factor> <numeric> <numeric>
1            1         0         0
2            2         0         0
3            3         0         0
4            4         0         0
5            5         0         0
...        ...       ...       ...
19          19  0.00e+00  0.00e+00
20          20  0.00e+00  0.00e+00
14          14  1.20e-13  1.33e-13
18          18  7.59e-09  7.99e-09
10          10  1.09e-02  1.09e-02
> warnings()
Warning messages:
1: In checkConv(attr(opt, "derivs"), opt$par, ctrl = control$checkConv,  ... :
  Model is nearly unidentifiable: very large eigenvalue
 - Rescale variables?

Identical warning message for 20 times. Please help.


Get report visuals data results

If there posible to get the data of visual objets of a report ?, i alreay creade de pbi.report() and is fine, but i need to create a script that get data for a visual object of that specific report

Unfortunately I don't think this data is available through the Rest API. If you knew the specifics of the visual, you could try using Dataset.execute_queries which lets you execute a DAX query against a Dataset. You could then run a DAX query that tries to replicate the data in the visual.

Unfortunately I don't think this data is available through the Rest API. If you knew the specifics of the visual, you could try using Dataset.execute_queries which lets you execute a DAX query against a Dataset. You could then run a DAX query that tries to replicate the data in the visual.

Thanks

@CrasCris I've experienced this error, solution is to use DAX query under visual as @andrewvillazon said above. If you don't have knowledge about DAX u can use autogenerated by PowerBI Desktop engine. You can find it in Performance analyzer pane. I used build in sample as example.


Incoming Webhook Security

Hey,
What is the best way (or alternatives) to have a secure incoming webhook? It looks like keeping webhook url a secret is the only way. But, if someone gets hold of the url then it can be heavily spammed or misused using Action Messages?
I see "Secure resource definitions" in the documentation and the limits to webhook posting. But is there a way to have a simple basic auth or any alternative to have this functionality in a more secure way?
Thanks,
Pankaj

@panjkov, Answer to your question is No. Check the documentations

The connector instance URL should be treated as secret/confidential: anyone who has that URL can post to it, like an email address. Therefore, there's some risk of spam or links to phishing or malware sites. If that were to happen, team owners can delete the connector instance.
If the service that sends connector messages were to become compromised and start sending spam/phishing/malware links, a tenant administrator can prevent new connector instances from being created and Microsoft can block them centrally.

@Abhijit-MSFT Thanks for this answer. Can Bot Framework be used to implement the features that we are looking for?

@pankajsharma2000 - You could create a new Office 365 Connector and disable actions.


add table of contents plugin to markdown-it

the only one i found that appears to be maintained is this one
it would used by adding [[toc]] anywhere in the markdown file like this:

# Heading 1

[[toc]]

...
....
...

Here is an example of how i did it on my fork. I'd need to add options in the _config.yaml for the plugin options. Right now they're hard coded.

How would you go about this?
Another option is to write my own toc generator and make it aa reptar plugin instead. I'd do it similarly to reptar-excerp. Then, on the file frontmatter, you can add an option toc: true.

what do you think ?

Oh this is a great idea!
I do agree that allowing this to be added via a plugin makes the most sense as then anyone can easily extend their installation and include any plugins that they want.
I'm trying to think of the proper API for this.
Right now my current thought is to have an Plugin.event.reptar.afterUpdate() function that will get run at the end of the update() command.
My main question now is what should be given to the plugin event that allows for customization. I'm half tempted to give the reptar instance, however that still won't give you access to the markdown instance. So that's the last piece of the puzzle to figure out.

Thank you. I will try to think about it as well in the meantime.

Got a flexible solution pushed out in version 2.3.0 https://github.com/reptar/reptar/blob/master/CHANGELOG.md#230-2016-11-06


Kentucky Onboarding - Build Receiver Settings

Acceptance Criteria

[ ] Added to organization.yml and created yamlitos

[ ] Customer status = testing

[ ] Added to prod settings table

[ ] Added quality filter = allowNone()

[ ] Added to staging settings table

[ ] Added quality filter = allowNone()

To Do

[ ] RS engineer: update this ticket with comments with every interaction with receiver
[ ] RS team member who originates this ticket: add any dependencies, as needed
[ ] RS team member who originates this ticket: complete the following when creating this Issue.

[ ] Pipelines: Onboarding & Operations (New)
[ ] Labels: "Onboarding-ops" and "Receiver" and state label
[ ] Epics: [select the epic for this API onboarding sender]

Next meeting set for 8/25/23

Happy Friday all,
Thank you so much for you willingness to answer our discovery team's questions and apologies for the disruption in the middle of our meeting as Zoom automatically ended the meeting after 40 minutes.
Notes from 7/28/23 Meeting:
At this time KHIE has not found any glaring legal issues that would prevent us from moving forward
KY DPH working with AIMS/APHL on S3 transfer as method for sending ELR/other public health data
This work is ongoing and KHIE would prefer to onboard ReportStream (RS) to this method as opposed to another method and moving to S3 transfer after the fact.
RS uses Azure blob storage not AWS S3 buckets, but methods for transfer between Azure/AWS are well documented and unlikely to be barrier (we didn't discuss this in the meeting but I thought I'd bring it up here just as an FYI)
Currently unclear on timeline for build-out, KHIE/ KY DPH still working on agreements with AIMS/APHL
Discrepancy between desired legal agreements between AIMS/KY and RS/KY is of some concern to KY DPH
KY DPH/KHIE to have internal meeting and discussion with legal council to determine path forward
KY DPH/KHIE to have discussions with AIMS/APHL on legal requirements after internal discussion
RS has senders signed terms of service prior to sending data to RS that is fairly broad in nature
Maurice Reaves (RS - Product Owner) - Current determination is that BAA/ signed legal agreement is not needed between CDC/RS and Public Health Agencies for public health reporting purposes
KY DPH question - Is there written guidance from HHS regarding this?
A: Not explicitly, but could be possible to obtain. Mo to check with HHS OGC (may take some time)
KY/KHIE requesting time to do discovery on legal and subsequent S3 work.
Check-in meeting to be set for 4 weeks.
Action Items:
James Gilmore (RS) - send invite for check in meeting
Maurice Reeves (RS) - Follow-up with HHS OGC to see if written documentation/opinion regarding unneeded agreement between RS/PHA can be produced
If obtained send to Kayla Rose (KHIE) and Courtney Marshall (KY DPH)
Please be on the lookout for an upcoming invite for 4 weeks from now and let me know if I have incorrectly captured anything or if there are any questions.
Thanks,
James

statement from Mo Reeves on KHIE requesting BAA. "We would like to have a meeting between the HHS Office of General Counsel and their lawyer. Let them know that right now we’re trying to set that up and find out who from their side on the legal end we should meet with."

Blocked by legal issues


Add build workflows GitHub actions

The following pull request adds the below github workdlows:
0ebaa2b Added building images on every tag
8e61ed7 Build and push image on every commit push
de1945e Update the build_and_test_midstream_image.yml to run only on pull request
9b09bf5 create build_and_test_midstream_image.yml github action

Example successful runs for the github actions is as follows:
https://github.com/samvarankashyap/operator-test-playbooks/actions

Looks like the actions get triggered only after they are merged.
However, you can see the example runs on my fork.

@dirgim I am not sure of the documentation because the github workflows are self explainatory and i added inline comments as much as possible to make it simple.

lgtm


Error: unable to build kubernetes objects from release manifest: [unable to recognize "": no matches for kind "ClusterRole" in version "rbac.authorization.k8s.io/v1beta1",

Contact Details (optional)
No response
Description. What happened?
The following error occurred to me, when terraform apply runs.
│ Error: unable to build kubernetes objects from release manifest: [unable to recognize "": no matches for kind "ClusterRole" in version "rbac.authorization.k8s.io/v1beta1", unable to recognize "": no matches for kind "ClusterRoleBinding" in version "rbac.authorization.k8s.io/v1beta1"]
│
│   with module.cloudwatch_logs.helm_release.cloudwatch_logs[0],
│   on .terraform/modules/cloudwatch_logs/helm.tf line 1, in resource "helm_release" "cloudwatch_logs":
│    1: resource "helm_release" "cloudwatch_logs" {

Steps to reproduce

Version information

$ terraform version
Terraform v1.1.7
on linux_amd64
+ provider registry.terraform.io/hashicorp/aws v3.70.0
+ provider registry.terraform.io/hashicorp/helm v2.1.2
+ provider registry.terraform.io/hashicorp/kubernetes v2.4.1
+ provider registry.terraform.io/hashicorp/tls v3.3.0

Prepare the following tf file

module "cloudwatch_logs" {
  source = "git::https://github.com/DNXLabs/terraform-aws-eks-cloudwatch-logs.git?ref=0.1.4"

  enabled = true
  # default
  namespace = "aws-cloudwatch-logs"
  # default
  service_account_name = "aws-for-fluent-bit"

  cluster_name                     = aws_eks_cluster.cluster.name
  cluster_identity_oidc_issuer     = aws_eks_cluster.cluster.identity.0.oidc.0.issuer
  cluster_identity_oidc_issuer_arn = aws_iam_openid_connect_provider.cluster.arn
  worker_iam_role_name             = aws_iam_role.node_iam_role.name
  region                           = var.region
}

data "tls_certificate" "cluster" {
  url = aws_eks_cluster.cluster.identity.0.oidc.0.issuer
}

resource "aws_iam_openid_connect_provider" "cluster" {
  client_id_list  = ["sts.amazonaws.com"]
  thumbprint_list = [data.tls_certificate.cluster.certificates.0.sha1_fingerprint]
  url             = aws_eks_cluster.cluster.identity.0.oidc.0.issuer
}

resource "aws_iam_role" "eks_cloudwatch_logs_role" {
  name = "eks_cloudwatch_logs_role"
  assume_role_policy = templatefile("policies/oidc_assume_role_policy.json", {
    OIDC_ARN = aws_iam_openid_connect_provider.cluster.arn
    OIDC_URL = replace(aws_iam_openid_connect_provider.cluster.url, "https://", ""), NAMESPACE = "kube-system",
    SA_NAME  = "aws-node"
  })
}

data "aws_eks_cluster" "cluster" {
  name = aws_eks_cluster.cluster.id
}
data "aws_eks_cluster_auth" "cluster" {
  name = aws_eks_cluster.cluster.id
}
provider "kubernetes" {
  host                   = data.aws_eks_cluster.cluster.endpoint
  cluster_ca_certificate = base64decode(data.aws_eks_cluster.cluster.certificate_authority[0].data)
  token                  = data.aws_eks_cluster_auth.cluster.token
  exec {
    api_version = "client.authentication.k8s.io/v1alpha1"
    args = [
      "eks", "get-token",
      "--cluster-name", aws_eks_cluster.cluster.name,
      "--region", var.region
    ]
    command = "aws"
  }
}

provider "helm" {
  kubernetes {
    host                   = data.aws_eks_cluster.cluster.endpoint
    cluster_ca_certificate = base64decode(data.aws_eks_cluster.cluster.certificate_authority.0.data)
    token                  = data.aws_eks_cluster_auth.cluster.token
    exec {
      # https://stackoverflow.com/questions/71318743/kubectl-versions-error-exec-plugin-is-configured-to-use-api-version-client-auth
      api_version = "client.authentication.k8s.io/v1alpha1"
      # With both v1beta1 and v1alpha1, this issue is not fixed
      #api_version = "client.authentication.k8s.io/v1beta1"
      args = [
        "eks", "get-token",
        "--cluster-name", aws_eks_cluster.cluster.name,
        "--region", var.region
      ]
      command = "aws"
    }
  }
}

Run terraform apply command

Expected behavior
Successful to apply Terraform resources
Actual behavior
Same as the description
Occurrence
Frequently
Relevant log output
No response
Code of Conduct

[X] I agree to follow this project's Code of Conduct

@gkzz Specifying the currently latest helm chart version on the module solved this issue for me:
helm_chart_version = "0.1.17"


feat: switch to file-based OLM catalogs

The Catalog index yaml file for the new file-based catalog will be maintained in the olm-catalog branch. The PR introduces the changes needed to generate new catalog index entry for new releases to the development, candidate, and the stable channel and to push the updated index to the olm-catalog git branch.
To switch to new file-based OLM catalogs, following changes have been made

upgrade to newer version of operator-sdk tools (operator-sdk, olm)

Generate catalog for existing 0.0.13 bundle by

creating new catalog index dockerfile
adding package definition for observability-operator * adding channel definitions for all 3 channels see: https://github.com/rhobs/observability-operator/pull/194

Modify GitHub workflow to adapt to the new file-based catalog

JIRA: https://issues.redhat.com/browse/MON-2791
Signed-off-by: Sunil Thaha<EMAIL_ADDRESS>

NOTE This requires the olm-catalog pr https://github.com/rhobs/observability-operator/pull/194 to be merged first so that when this PR merges, olm-publish action that is run after a commit merges to main runs fine is able to publish catalog images to quay.
The olm-publish action makes the assumption that the branch - olm-catalog exists and that it contains the catalog index.


CI should check that we don't build rustc_codegen_spirv more than once.

See #546 for some context and ideas - especially https://github.com/EmbarkStudios/rust-gpu/pull/546#issuecomment-809089160.
Sadly, this is likely blocked on moving CI to use --release to build examples (which depend on "build dependency release mode rustc_codegen_spirv via the spirv-builder-using build scripts), otherwise the problem can't be reproduced in the first place (AFAIK).

Superseded by #911


consider building tokio-tls and openssl optionally

@stepancheg
I see that now grpc imports tokio-tls and openssl but doesn't use it anywhere. Users should install openssl to build even they don't want to use TLS.
Maybe we can use a optional feature to control it.

I agree.

/cc @overvenus
Maybe we can send a PR later.

I would recommend that if TLS is made optional, the default should be to have it operational. We should encourage the use of TLS.

Probably better option would be making TLS implementation optional in native-tls crate. I've created a feature-request.

Looks like that issue was closed unfortunately :(


How to show headers for a single request?

There's currently no way to do this, except by editing the blade views or generated markdown files. However, we're beginning work on v3, so we're open to ideas on how we can incorporate this (if it doesn't add cognitive load to the user)

This is going to be supported in v3 by specifying 'apply' rules for route groups


OpenAI fine tuning for boolean metrics

This is not necessarily mergeable but it shows how to actually do the fine-tune in TypeScript. We will likely want to add some tests and also determine how to build a broader web application to enable this more easily.
But, I think it's worth reviewing at this point and running to make sure you can. Let's sync up on how to incorporate this into the plan. I can now easily fill out the rest of feature-parity for fine-tuning here.

Final PR #520


add inst mutating docstring

Add a clarifying docstring about inst causing mutations. This caused a bug in the generation of programs for QAOA a few weeks back.

👍 LGTM.  More documentation the better.


postcss integration

Could you integrate @postcss into msbuild?

Team triage: we have no plans to do this in core MSBuild. The best way to go about it would be to create a NuGet package that delivers targets and tasks to call into postcss tools.