Weird touch behavior on mobile device

I found problems on mobile device and chrome developer tool's mobile device mode. The video is recorded on chrome on macbook.
This happpens in https://desk-x.com and your demo mvlabat.github.io/bevy_egui_web_showcase, and does not happens in https://egui.rs
Problems

Drag position is reset on any next touch.
It can be moved by dragging any position.

Possible solution
Proper handling of touch release event (or something).
https://user-images.githubusercontent.com/8780513/167227716-505e70e3-7907-4a2c-9399-5814fc4bfc5e.mov

Hi! Thanks for filing an issue. Yeah, unfortunately, bevy_egui doesn't have a proper touch device support, as it doesn't react to touch events at all.  I'll be happy to accept a PR if you'd like to tackle this.

I'd like to resolve this. Is it hard to do? just not yet implemented?

I think it shouldn't be difficult. I believe the only system you'll need to look into is process_input. But since I've never worked with touch devices, I've no idea how many edge-cases there can be. :) It can be also a good idea to look into the egui's demo implementation (I believe its desktop version also works on winit as does Bevy).

Ok. Thanks. I’ll try it.

The input event of bevy is from winit.
But winit hasn't implemented touch handling on web target. https://github.com/rust-windowing/winit/issues/1673
eframe get touch event from web_sys.
So, should we include web_sys dependency to support touch?

I forgot about my PR that adds support web touch event to winit.
https://github.com/rust-windowing/winit/pull/2188
I think waiting for merge is fine for me.

https://github.com/rust-windowing/winit/pull/2188
Things advances!

Wow, nice! If web touch device support lands into winit, that will be great. Thank you for your work on this @ryo33.
@71e6fd52 if anyone wants to make a PR adding touch device support by using web_sys directly, I'm open to merging it. I'd probably just feature gate it. Once the winit PR is merged and Bevy upgrades to a newer version, we can always switch to the native support.

I use winit with the PR 2188, and I modified bevy_egui to handle touch event, I got this, which seems to work, by inspiring from bevy_winit.
Here is the branch: https://github.com/thiolliere/bevy_egui/tree/gui-touch-event
The diff: https://github.com/mvlabat/bevy_egui/compare/main...thiolliere:bevy_egui:gui-touch-event
diff --git a/src/systems.rs b/src/systems.rs
index e484397..a0ed473 100644
--- a/src/systems.rs
+++ b/src/systems.rs
@@ -13,6 +13,7 @@ use bevy::{
         keyboard::{KeyCode, KeyboardInput},
         mouse::{MouseButton, MouseButtonInput, MouseScrollUnit, MouseWheel},
         ElementState, Input,
+        touch::TouchInput,
     },
     prelude::EventReader,
     utils::HashMap,
@@ -33,6 +34,7 @@ pub struct InputEvents<'w, 's> {
     ev_keyboard_input: EventReader<'w, 's, KeyboardInput>,
     ev_window_focused: EventReader<'w, 's, WindowFocused>,
     ev_window_created: EventReader<'w, 's, WindowCreated>,
+    ev_touch: EventReader<'w, 's, TouchInput>,
 }

 #[derive(SystemParam)]
@@ -40,6 +42,7 @@ pub struct InputResources<'w, 's> {
     #[cfg(feature = "manage_clipboard")]
     egui_clipboard: Res<'w, crate::EguiClipboard>,
     keyboard_input: Res<'w, Input<KeyCode>>,
+    pointer_touch_id: Local<'s , Option<u64>>,
     egui_input: ResMut<'w, HashMap<WindowId, EguiInput>>,
     #[system_param(ignore)]
     marker: PhantomData<&'s usize>,
@@ -225,6 +228,8 @@ pub fn process_input(
         }
     }

+    let focused_window_height = window_resources.focused_window.as_ref().map(|window_id| window_resources.window_sizes[&window_id].height());
+
     if let Some(focused_input) = window_resources
         .focused_window
         .as_ref()
@@ -269,6 +274,77 @@ pub fn process_input(
         }

         focused_input.raw_input.modifiers = modifiers;
+
+        for touch in input_events.ev_touch.iter() {
+            let scale_factor = egui_settings.scale_factor as f32;
+            let mut touch_position: (f32, f32) = (touch.position / scale_factor).into();
+            touch_position.1 = focused_window_height.unwrap() / scale_factor - touch_position.1;
+
+            // Emit touch event
+            focused_input.raw_input.events.push(egui::Event::Touch {
+                device_id: egui::TouchDeviceId(egui::epaint::util::hash(touch.id)),
+                id: egui::TouchId::from(touch.id),
+                phase: match touch.phase {
+                    bevy::input::touch::TouchPhase::Started => egui::TouchPhase::Start,
+                    bevy::input::touch::TouchPhase::Moved => egui::TouchPhase::Move,
+                    bevy::input::touch::TouchPhase::Ended => egui::TouchPhase::End,
+                    bevy::input::touch::TouchPhase::Cancelled => egui::TouchPhase::Cancel,
+                },
+                pos: egui::pos2(touch_position.0, touch_position.1),
+                force: match touch.force {
+                    Some(bevy::input::touch::ForceTouch::Normalized(force)) => force as f32,
+                    Some(bevy::input::touch::ForceTouch::Calibrated {
+                        force,
+                        max_possible_force,
+                        ..
+                    }) => (force / max_possible_force) as f32,
+                    None => 0_f32,
+                },
+            });
+
+            // If we're not yet tanslating a touch or we're translating this very
+            // touch …
+            if input_resources.pointer_touch_id.is_none() || input_resources.pointer_touch_id.unwrap() == touch.id {
+                // … emit PointerButton resp. PointerMoved events to emulate mouse
+                match touch.phase {
+                    bevy::input::touch::TouchPhase::Started => {
+                        *input_resources.pointer_touch_id = Some(touch.id);
+                        // First move the pointer to the right location
+                        focused_input.raw_input
+                            .events
+                            .push(egui::Event::PointerMoved(egui::pos2(touch_position.0, touch_position.1)));
+                        // Then do mouse button input
+                        focused_input.raw_input.events.push(egui::Event::PointerButton {
+                            pos: egui::pos2(touch_position.0, touch_position.1),
+                            button: egui::PointerButton::Primary,
+                            pressed: true,
+                            modifiers,
+                        });
+                    }
+                    bevy::input::touch::TouchPhase::Moved => {
+                        focused_input.raw_input
+                            .events
+                            .push(egui::Event::PointerMoved(egui::pos2(touch_position.0, touch_position.1)));
+                    }
+                    bevy::input::touch::TouchPhase::Ended => {
+                        *input_resources.pointer_touch_id = None;
+                        focused_input.raw_input.events.push(egui::Event::PointerButton {
+                            pos: egui::pos2(touch_position.0, touch_position.1),
+                            button: egui::PointerButton::Primary,
+                            pressed: false,
+                            modifiers,
+                        });
+                        focused_input.raw_input.events.push(egui::Event::PointerGone);
+                    }
+                    bevy::input::touch::TouchPhase::Cancelled => {
+                        *input_resources.pointer_touch_id = None;
+                        focused_input.raw_input.events.push(egui::Event::PointerGone);
+                    }
+                }
+            }
+
+        }
+
     }

     for egui_input in input_resources.egui_input.values_mut() {

Also thanks for the good work

https://github.com/rust-windowing/winit/pull/2188 is steady progress.
@thiolliere Nice work! Could you submit a pull-req? I plan to use this feature for my product which will be released next month!

Can I close this? #134 has been opened, and this cannot be resolved until winit merges it.

https://github.com/rust-windowing/winit/pull/2188 have been merged!

@ryo33 nice work! Glad it has been merged :)
I don't mind keeping this issue open btw, to track touch device support in web specifically.


Versioning should be visible in the footer, and issues tagged with the version that it was seen in

I fixed this today.


[Impeller] Investigate alternatives to using the swapchain with Vulkan on Android.

From the last Skia Graphics sync (go/impeller-graphite-sync), it was mentioned that Chrome uses surface control to create and send Android Hardware Buffers directly to SurfaceFlinger when able. Since there is an API availability issue, there is a Vulkan swapchain fallback.
This technique may also be used for GL [citation needed, see chat thread about the mention].
The main benefit to avoiding swapchains was that they make it hard to control allocations for the underlying images and image views.  This tracks as Impeller goes to great lengths (relative to other backends) avoid hitting the allocators in frame workloads.
From discussion in go/impeller-weekly, work on this alternative will not be prioritized till we are past stable with Impeller on Android.
References & Breadcrumbs

https://developer.android.com/ndk/reference/group/native-activity#asurfacecontrol
https://developer.android.com/ndk/reference/group/a-hardware-buffer
Chat thread go/bc94d8bd-bfcf-4915-b7d1-8d5d213eeb55
Email Thread go/15ec12be-dfda-4ce7-8e39-20db4ebedd50

Duplicate of https://github.com/flutter/flutter/issues/143105


server dist-ossec-keys.sh script not working

hi @jtimberman ,
I have a question about the dist-ossec-keys.sh script.
here it is in my vm :
cat /usr/local/bin/dist-ossec-keys.sh
#!/bin/sh
#

for host in chef-production2-doctor-simulator01
do
  echo "host is $host"
  key=`mktemp`
  grep $host /var/ossec/etc/client.keys > $key
  scp -i /var/ossec/.ssh/id_rsa -B -o UserKnownHostsFile=/dev/null -o StrictHostKeyChecking=no $key ossecd@$host:/var/ossec/etc/client.keys >/dev/null 2>/dev/null
  rm $key
done

I ran it and I don't quite understand what it's doing , if anything at all.
please help

here is
cat /var/ossec/etc/client.keys
001 #*#*#*#*#*#*#*#*#*#*#197.221.226 7196c76c568258e2ad836f8e1aa37e0758dee969f560ceb59be76879c3f3412d
002 test-agent <IP_ADDRESS> e2c9b117f26a202598007dcb4ec64e01f18000f9d820f6b3508a95e5313e6537

Closing due to inactivity.
If this is still an issue please reopen or open another issue. Alternatively drop by the #sous-chefs channel on the Chef Community Slack and we'll be happy to help!
Thanks,
Sous-Chefs


iPhone X on iOS 12.1.0 (16B92) crash in prod app

I have crash in prod app, unfortunately not so much info, only logs from the firebase crash.
Crashed: com.apple.main-thread
0  libobjc.A.dylib                0x20f43eb00 objc_object::release()
1  libobjc.A.dylib                0x20f43f38c (anonymous namespace)::AutoreleasePoolPage::pop(void*)
2  CoreFoundation                 0x2101e4804 __CFRUNLOOP_IS_CALLING_OUT_TO_A_TIMER_CALLBACK_FUNCTION__
3  CoreFoundation                 0x2101e4534 __CFRunLoopDoTimer
4  CoreFoundation                 0x2101e3d68 __CFRunLoopDoTimers
5  CoreFoundation                 0x2101dec44 __CFRunLoopRun
6  CoreFoundation                 0x2101de1cc CFRunLoopRunSpecific
7  GraphicsServices               0x212455584 GSEventRunModal
8  UIKitCore                      0x23d311054 UIApplicationMain
9  Runner                         0x1046822a8 main (AppDelegate.swift:8)
10 libdyld.dylib                  0x20fc9ebb4 start

here is the flutter doctor output:
[✓] Flutter (Channel stable, v1.0.0, on Mac OS X 10.13.6 17G4015, locale en-US)
    • Flutter version 1.0.0 at /Users/romatroskin/Developer/flutter
    • Framework revision 5391447fae (4 weeks ago), 2018-11-29 19:41:26 -0800
    • Engine revision 7375a0f414
    • Dart version 2.1.0 (build 2.1.0-dev.9.4 f9ebf21297)

Hope it will be helpful somehow :)

Is this from a release build?
Any information about what action led to the crash, the used device, ...?

I had the same issue before in 1.0 version. Now I try to upgrade to lastest(1.2.1) and take a look again.

I had the same issue.
Flutter version 1.2.1


Include in comparation to work with list

It would be a good improvement, to contemplate the in operation to work with lists and tuples.
Example:
rule = {"in" : [ { "var" : "temp" }, 110 ]}

data = { "temp" : [99, 100, 101]}

jsonLogic(rules, data)
# True

https://github.com/nadirizr/json-logic-py/pull/13

@ainokila I think this a bit off the way "in" is designed to work.
Think about this as:
"n in m", where m must be always an iterable
and n is user data
what you suggest is:
"n contains m"?
Im also not sure how
[99, 100, 101] in 110 might produce True as output ?


Elaine/bug fixes #94

Fixes bug described in issue #94

@elaineo
Great job.
viewed, function ok now.


Fetch Data from Firebase Realtime Database in Descending Order[Feature Request]

Is it possible to fetch data using RealtimeDBPagination in Descending Order?
Basically I want my latest data to be on the top of the screen and older to the bottom of the screen.
A walkthrough would be appreciated.
Thanks,

changed _data with _data.toList().reversed.toList() works fine for me. We can ask if data should be reversed in the widget and define it as
reverseData ? _data.toList().reversed.toList() : _data

@CodeWithBishal, yeah the problem is Firebase Realtime Database doesn't support decending orderBy.
I'm quite busy right now, so PRs are welcomed.
Just heads up
I'm currently not actively maintaining this particular package. So instead I would suggest using the official firebase pagination package for realtime database.


Merge default options with provided options and provide defaults for pug

Thank you for your efforts with svelte-preprocess. I'm exploring svelte right now and prefer sass and pug. Your repo saved me the time and effort of writing my own hooks to preprocess. 👍
Having include paths for sass and stylus is good, but unfortunately I was finding the default code paths passed in an empty options object (in utils.js/runTransformer()). So, the include paths aren't added by default. This PR merges in the default options with Object.assign(). (We could reuse the options variable. I opted for a new opts variable for clarity but can change it.)
This PR also plumbs through sensible defaults for pug - the filename (fixes imports) and html as the default doctype (fixes boolean attributes).
Thanks again!

Oh, thank you! I was meaning to fix that default options issue 😁

@kaisermann thank you for your quick response and your work with svelte!


Clicking a point on a Plots.jl plot

You click somewhere on the plot, and it returns the clicked coordinates in the plot coordinate space.
Maybe as a package extension?

And the API will be a new function: PlotCoordinatePicker. Because #3 will already dispatch ImageCoordinatePicker on a Plots.Plot and give the image-space coordinates.


Dependency cleanup

Remove unnecessary dependencies.

Note: imageio block was removed because:

napari 0.4.17 is released with the blocking patch
imageio has a new release

@talonchandler Please refer to the description of https://github.com/mehta-lab/recOrder/pull/283/commits/347e6fb5c691514c138f4d1fe2b4f4bb6bc85f4b and napari's requirements for your concerns.

@talonchandler Please refer to the description of 347e6fb and napari's requirements for your concerns.

Thanks for the links and for looking into this!


feat(protractor): isPresent() now is friendly to out of bounds errors

fixes #2917
@juliemr worked around your comments in that issue,could you check these changes?. Modified ElementArrayFinder.prototype.get function, now it does't crash the flow when an out of bounds err is thrown, instead of that a warning is prompted in console.

@sjelin I understand your point, but if .get fails there is no friendly response with another actions like isPresent() even if the test passes, now it will crash when operations like click() in undefined elements but there will be the warning in the console explaining the out of bounds error.

I think people are less likely to see a warning logged to the console than an error thrown at the time they use .get.  We could add an optional variable to .get called opt_warnOnOutOfBounds or something for users who want to do elems.get(i).isPresent().
@juliemr @cnishina @mgiambalvo

I was thinking on an approach like that with an optional variable but I tought it was going to be the same result at the end. The flow of the test will crash and show the out of bounds error.
But yeah that would be a nice approach.

Can't we have isPresent check for the rejected promise, intercept it, and do the right thing? No need for any boolean params.

Good idea Julie.  See #3108


Improve zip / low accuracy behavior or messaging

Accepting zips as location is important for simplicity; folks like to have a low-PII way to provide their location. However, zips don't map to congressional districts in all cases and our mapping just uses the district that the "center" of a zip (as determined by google maps API) lands in.
We could do a better job of communicating when this happens, in some or all cases. Some ideas:

Process all zip polygons and find out exactly which ones map perfectly to congressional districts and which ones are ambiguous
When we detect a zip used as a location input (or just an ambiguous zip if we do the above step), present a message in the location picker that indicates this is an estimate only. Perhaps users can preview the house rep that we detected and if they know it's wrong, use an address? or pick from a list of reps? I don't expect many people will know exactly who to pick, so I don't really want to put the effort back on the user.

I like the idea of having a message that this is an estimate (especially if it only shows for ambiguous zip codes). I'd like users to be able to test drive the app without committing to providing personal info. Then make it as easy as possible to update/correct once they decide they'd like to use the app for real.
Do you have a feel for what percentage of zip codes are ambiguous?

I have not run the numbers on the spatial data we have, but a number of years ago I read that 30% of zips are split. But some of those will return correct districts, some of those are probably very low population zips and that's before the more recent 2020 redistricting, so it's hard to tell how much it impacts folks in reality.
I captured details here because I was thinking about it but it's not something I want to focus on for the initial 2.0 release.


Fix search issue with author and dedup_titles both selected

There's an odd issue with PyMISP's search_index() function where it either truncates the search term at | or treats | as an OR statement.
Either way, if you have a large DB filled with stuff like "Alienvault | Pulse Name" you'll get back results for every event that has Alienvault in the title. That is, unless you changing the | to | right before searching, which is what this PR does.

Just out of curiosity, does this defect actually reveal a SQL injection vulnerability in the MISP search feature? I believe that search strings should never be parsed, so special characters like pipe (|) should have no effect if untrusted data is treated appropriately.


Update specs spreadsheet and add Inches column to tables

Adds the new "Inches" column from the spreadsheet. I also re-exported the spreadsheet.

Closing in favor of #323.


Fix duplicate swissprot and trembl proteins

Mygene has 19326 genes which encode one or more proteins. There are 27625 items in wikidata that are found in taxon homo sapiens, and have a uniprot ID.
Examples:

https://www.wikidata.org/wiki/Q21117550, which is the TrEMBL version of a protein (this: https://www.wikidata.org/wiki/Q3046953), but has no links to it, no encoded by statements and nothing encodes it. Should probably merge these?
Items like this: CAV1 (gene), encodes a swissprot and a trembl version of the same protein. I'm not sure why this was done, especially because there are 8 trembl entries ("A0A024R757","A9XTE5","C9JKI3","E9PCT5","F8WDM7","Q2TNI1","Q59E85","Q7Z4F3") (mygene). I don't see the use in having both of these. Should be merged.

ad 1: There is no encodes info bc the Trembl entry has no link to ncbi gene. I would not merge that with the swissprot entry, I'd rather delete it. bc if you merge it, you will drag in wrong info (e.g. automatically generated GO annotations, an irrelevant Unprot id which will not match the GO annotation after the next protein bot run, etc) onto existing Swissprot items.
ad 2: Seems like that happened due to a bot glitch on 2015-10-15, I somehow remember this had to do with use of label matching instead of ID matching. When I took responsibility for protein bot, a fast fix was required so I just linked all which could be linked to a gene. 2451 are still not linked. As I described above, I'd not merge these, but rather delete them.

What about just deleting all 7282 protein items that have "annotated by UniProtKB/TrEMBL" in their description? (query)

Submitted the batch of 2125 trembl proteins with no encodes or encoded by:
https://www.wikidata.org/wiki/Wikidata:Requests_for_deletions#TrEMBL_Human_Proteins
Let's see if anyone objects, then will do the others

They seem to think these items are better off merged...
https://www.wikidata.org/wiki/Wikidata:Requests_for_deletions#Why_was_my_request_removed.3F

After some convincing by ChristianKl, they've been deleted.
That still leaves about ~5k human proteins that have an encodes or encoded by, that need to be cleaned up at some point.
http://tinyurl.com/y7opp338
Steps to clean up:

Remove all "encoded by" statements from these proteins
Remove all "encodes" statements on genes that link to these proteins
Submit for deletion


Change build rules to stop mobile and fixed-line descs inheriting from the generalDesc element

Metadata changes:
-- Drops the "-1" that was erroneously included in possible lengths before (didn't break anything, but was wrong) - it was a possibleLength of a sub-component so got added to the generalDesc possibleLengths
-- possibleNumberPattern no longer inherited: we don't use this anyway, we will do another CL soon to stop including it at all in the generated metadata
-- exampleNumber is no longer set on fixed-line and mobile elements from the generalDesc
XML file changes:
-- Stopped specifying "NA" and "-1" specifically for fixed-line and mobile blocks; now they are treated as every other type of phone number: if missing, don't fill them in from generalDesc, but leave them missing.
Code changes:
-- Stop using the exampleNumber on generalDesc for non-geo entities, but look at their phonenumber descs - the exampleNumber won't be stored on the generalDesc anymore. This affects porters if they either copied our build logic or used our built metadata in some way; they should update this method in their port too.
************** Do not copy into final commit **************
See https://github.com/googlei18n/libphonenumber/pull/1422 for original review.
Taking over as 2 of the followup commits there were committed with non-CLA addresses although author has signed CLA with other addresses.
Jar diffs:
~keghani/GitHub_diffs/PR1427/pkgdiff_reports/master_cpp/build_to_build/changes_report.html
~keghani/GitHub_diffs/PR1427/pkgdiff_reports/master_java/build_to_build/changes_report.html

This change is

I'd copy and paste the original commit message rather than having the "See ..."

Actually since I'm doing this, I'll also update the jars in this PR.
Please don't merge! Feel free to look though, it'll just be another commit with 2 jars. Diff coming up.

Done re. PR description - this doesn't have to be the final commit message though.

Jar diffs:
~keghani/GitHub_diffs/PR1427/pkgdiff_reports/master_cpp/build_to_build/changes_report.html#InformationFiles
~keghani/GitHub_diffs/PR1427/pkgdiff_reports/master_java/build_to_build/changes_report.html
All ready!

Reviewed all non-binary diffs. LGTM

Thanks ;)

nov. 2016 3.56 p.m. skrev "Keghani Kouzoujian"<EMAIL_ADDRESS>

:

Merged #1427 https://github.com/googlei18n/libphonenumber/pull/1427.
—
You are receiving this because you are subscribed to this thread.
Reply to this email directly, view it on GitHub
https://github.com/googlei18n/libphonenumber/pull/1427#event-851602877,
or mute the thread
https://github.com/notifications/unsubscribe-auth/AIjis72dZXxNLzVsa0ttspuPSYgdqJc9ks5q8I2lgaJpZM4KsfSC
.


config.loadPrivateKey is not a function

Hey,
TikkieCheckoutConfig does not have a method loadPrivateKey?

You removed it here https://github.com/ToonvanStrijp/node-tikkie-checkout/commit/a66dcf15b5dc595d85e3b6b19def5a931fdce2e0#diff-7e82dc7533620e594f349d3ea425b5b4L39, so I guess the example needs to be updated?

And also it looks like the API calls require an Authorization header?
https://developer.abnamro.com/api/tikkie-v1/technical-details

@yooouuri indeed the example need to be updated.
I think you got this library/api mixed up with https://github.com/DanielHuisman/node-tikkie
This library is an api client for the tikkie fast checkout api. This api does not require an Authorization header.


chore: update Noir version

Updates to the new version of Noir.
Checklist:
Remove the checklist to signal you've completed it. Enable auto-merge if the PR is ready to merge.

[ ] If the pull request requires a cryptography review (e.g. cryptographic algorithm implementations) I have added the 'crypto' tag.
[ ] I have reviewed my diff in github, line by line and removed unexpected formatting changes, testing logs, or commented-out code.
[ ] Every change is related to the PR description.
[ ] I have linked this pull request to relevant issues (if any exist).

This is giving a different error than on mainframe, though note that on mainframe I get:

CC @Maddiaa0 and @benesjan since it seems relevant to you folks

This is giving a different error than on mainframe, though note that on mainframe I get:

CC @Maddiaa0 and @benesjan since it seems relevant to you folks
Instructions:

Checkout this branch
Run ./bootstrap.sh in noir-contracts

@kevaundray The compute_note_hash_and_nullifier check is an untested code (we don't yet have a good way to test it) so it's quite likely it's just broken. I will investigate it on. monday.

Benchmark results
Metrics with a significant change:

note_trial_decrypting_time_in_ms (8): 3.00 (-97%)
note_trial_decrypting_time_in_ms (32): 84.0 (+38%)

Detailed results
All benchmarks are run on txs on the Benchmarking contract on the repository. Each tx consists of a batch call  to create_note and increment_balance, which guarantees that each tx has a private call, a nested private call, a public call, and a nested public call, as well as an emitted private note, an unencrypted log, and public storage read and write.
This benchmark source data is available in JSON format on S3 here.
Values are compared against data from master at commit db67aa1e and shown if the difference exceeds 1%.
L2 block published to L1
Each column represents the number of txs on an L2 block published to L1.

Metric
8 txs
32 txs
128 txs

l1_rollup_calldata_size_in_bytes
45,444
179,588
716,132

l1_rollup_calldata_gas
222,960
868,244
3,449,660

l1_rollup_execution_gas
842,047
3,595,352
22,205,029

l2_block_processing_time_in_ms
2,033 (+1%)
7,624
29,983 (+1%)

note_successful_decrypting_time_in_ms
298 (+3%)
876 (+3%)
3,216

note_trial_decrypting_time_in_ms
:warning: 3.00 (-97%)
:warning: 84.0 (+38%)
135 (-1%)

l2_block_building_time_in_ms
13,301 (+1%)
52,636 (+1%)
210,672 (+1%)

l2_block_rollup_simulation_time_in_ms
11,951 (+1%)
47,341 (+1%)
189,375 (+1%)

l2_block_public_tx_process_time_in_ms
1,302 (+2%)
5,158 (+1%)
20,795 (+1%)

L2 chain processing
Each column represents the number of blocks on the L2 chain where each block has 16 txs.

Metric
5 blocks
10 blocks

node_history_sync_time_in_ms
21,609
42,054

note_history_successful_decrypting_time_in_ms
1,991 (-2%)
3,895 (-2%)

note_history_trial_decrypting_time_in_ms
120 (-1%)
142 (-24%)

node_database_size_in_bytes
1,628,995
1,099,922

pxe_database_size_in_bytes
29,748
59,307

Circuits stats
Stats on running time and I/O sizes collected for every circuit run across all benchmarks.

Circuit
circuit_simulation_time_in_ms
circuit_input_size_in_bytes
circuit_output_size_in_bytes

private-kernel-init
72.3
61,697
18,841

private-kernel-ordering
43.5
24,233
8,089

base-rollup
1,773
656,309
811

root-rollup
77.5
4,072
1,097

private-kernel-inner
53.1
81,504
18,841

public-kernel-private-input
41.6
41,455
18,841

public-kernel-non-first-iteration
26.6
41,497
18,841

merge-rollup
0.913 (+1%)
2,592
873

Miscellaneous
Transaction sizes based on how many contracts are deployed in the tx.

Metric
0 deployed contracts
1 deployed contracts

tx_size_in_bytes
8,723
27,240


azure-storage-azcopy: 10.20.1 -> 10.21.1

Automatic update generated by nixpkgs-update tools. This update was made based on information from https://github.com/Azure/azure-storage-azcopy/releases.
meta.description for azure-storage-azcopy is: The new Azure Storage data transfer utility - AzCopy v10
Updates performed

Golang update

To inspect upstream changes

Release on GitHub

Compare changes on GitHub

Impact

Checks done (click to expand)

built on NixOS
The tests defined in passthru.tests, if any, passed
found 10.21.1 with grep in /nix/store/sc2w7gk1w0yv9ihdz1ydcjl5gbarp6lz-azure-storage-azcopy-10.21.1
found 10.21.1 in filename of file in /nix/store/sc2w7gk1w0yv9ihdz1ydcjl5gbarp6lz-azure-storage-azcopy-10.21.1

Rebuild report (if merged into master) (click to expand)

1 total rebuild path(s)

1 package rebuild(s)

First fifty rebuilds by attrpath
azure-storage-azcopy

Instructions to test this update (click to expand)

Either download from Cachix:
nix-store -r /nix/store/sc2w7gk1w0yv9ihdz1ydcjl5gbarp6lz-azure-storage-azcopy-10.21.1 \
  --option binary-caches 'https://cache.nixos.org/ https://nix-community.cachix.org/' \
  --option trusted-public-keys '
  nix-community.cachix.org-1:mB9FSh9qf2dCimDSUo8Zy7bkq5CX+/rkCWyvRCYg3Fs=
  cache.nixos.org-1:6NCHdD59X431o0gWypbMrAURkbJ16ZPMQFGspcDShjY=
  '

(The Cachix cache is only trusted for this store-path realization.)
For the Cachix download to work, your user must be in the trusted-users list or you can use sudo since root is effectively trusted.
Or, build yourself:
nix-build -A azure-storage-azcopy https://github.com/r-ryantm/nixpkgs/archive/2cc491405a0a98a94b6090968e20d91e83920651.tar.gz

Or:
nix build github:r-ryantm/nixpkgs/2cc491405a0a98a94b6090968e20d91e83920651#azure-storage-azcopy

After you've downloaded or built it, look at the files and if there are any, run the binaries:
ls -la /nix/store/sc2w7gk1w0yv9ihdz1ydcjl5gbarp6lz-azure-storage-azcopy-10.21.1
ls -la /nix/store/sc2w7gk1w0yv9ihdz1ydcjl5gbarp6lz-azure-storage-azcopy-10.21.1/bin

Pre-merge build results
We have automatically built all packages that will get rebuilt due to
this change.
This gives evidence on whether the upgrade will break dependent packages.
Note sometimes packages show up as failed to build independent of the
change, simply because they are already broken on the target branch.
Result of nixpkgs-review run on x86_64-linux 1

1 package built:

azure-storage-azcopy

Maintainer pings
cc @colemickens @kashw2 for testing.

Result of nixpkgs-review pr 268030 run on x86_64-linux 1

1 package built:

azure-storage-azcopy


image noise floor subtraction

The noise floor of linescans may differ slightly by channel.
Add an option to toggle image noise floor subtraction before curves are generated.

Subtracting the 20 percentile floor has virtually no effect on the shape of the ΔG/R curve
Because green floor is slightly larger than red floor, enabling subtraction increases ΔG/R by a few percent


MRTK Input system does not bubble events

Describe the bug
interactable.cs is not allowing raycast to hit boundingbox.cs object while I am trying to use both.
I have a requirement to rotate a parent object by dragging a child gameobject which has interactable.cs. For this to achieve, I have used boundingbox example of tree given in example scenes.
To reproduce
Steps to reproduce the behavior:

Go to bounding box examples scene where we have "tree/coffee cup/cheese etc"
select child of tree model named "node_id30"
put layer of "node_id30" to "IgnoreRaycast"
Go to child object of "node_id30" which is text and add a "box collider" component to it and "interactable.cs"  component.
Add a tag to text object as "HelixButton" (to detect the child object in boundingbox.cs)
Open Boundingbox.cs component of "node_id30" and add the code to the method OnPointerDown as shown in "fig1"

Expected behavior
I have used Focus and FocusOff functions for child gameobject (text). Hence the expected behavior is that the child object's interactable behaviour should work along with rotation of parent object when dragged by focusing on child object.
I am able to rotate parent gameobject by dragging child gameobject if I disable interactable.cs component of child gameobject. But not able to rotate if I enable ineractable.cs of child object
Screenshots
Your Setup (please complete the following information)

Unity Version [2019.1.10f1]
MRTK Version [v2.0]

Target Platform (please complete the following information)

HoloLens
HoloLens 2

@julenka @CDiaz-MS @thalbern @MenelvagorMilsom what is the general consensus here regarding the "stackability" of the various types of manipulation and interaction that we have in the MRTK?
AFAIK the model really is currently a "the first one to handle it wins" though it seems like in each case it probably wouldn't be that "hard" to update this so that things can be configured to not mark input events as handled, allowing things to bubble up and then chain.
That being said, this does mean that the whole interaction models would probably need to be defined (i.e. what does it mean to have a manipulation handler within a bounding box within an interactable)

@rupeshpamaihgari I suspect that you might be able to work around this in interactable by removing eventData.Use(); calls (which will allow events to keep going up) but obviously only on the thing that you want to not eat the input event.
The greater question of the overall philosophy/reasoning still needs to be enumerated.

I read through the description a few times and to be honest still am not fully understanding the use case. Could you maybe share an example scene, or a git branch, or maybe share a video of the expected and actual behavior? Code is always best. I think the default behavior to have interactable consume events is probably what we want to keep. If I can understand your repro better, it will help me come up with a workaround.

I read through the description a few times and to be honest still am not fully understanding the use case. Could you maybe share an example scene, or a git branch, or maybe share a video of the expected and actual behavior? Code is always best. I think the default behavior to have interactable consume events is probably what we want to keep. If I can understand your repro better, it will help me come up with a workaround.

@julenka Please find link   below for videos explaining behavior "Rotation working when interactables disabled" and "Rotation  not working when interactables enabled".

Ah okay, thank you, I understand now. MRTK has an unusual way of dispatching events, which is causing the problem. The event dispatch will send events until it hits an object that handles the event. In this case, Interactable is below bounding box in the hierarchy, so it handles the event first. The expected behavior would be for the event to continue to propogate upward unless some event handler calls "eventData.use()" to force the propogation to stop.
Graphically, I believe the expected behavior that rupeshpamaihgari has is:

This also matches the expected behavior for input dispatch systems such as how JavaScript dispatches mouse events
However, MRTK does this:

Thinking more on this, I think it would be better for us to follow common patterns in UI systems and bubble up events until they are explicitly used / stopped.

Thanks @julenka for considering this issue. I just have to achieve very basic interaction "rotating parent gameobject by dragging the child gameobject without using boundingbox.cs". It would be really helpful if there is any documentation or code for it.

assigning to @vaoliva as this will be needed for scrolling object collection to work properly

@elbuhofantasma, this should be fixed based on the PR that was merged. Can you confirm?

@polar-kev That PR was merged into a feature branch which hasn't yet made its way into the main branch.

As we move onto XRI, we're moving the handling of input events onto Unity's side of things. The feature branch will still be available here, and we'd love it if community members took and interest in refreshing this PR for the v2 input system.
https://github.com/microsoft/MixedRealityToolkit-Unity/tree/feature/event_propagation


FEATURE: RB-53 Support ecommerce-demo for AIO

Build succeeded (check pipeline).

https://checklist.intgdc.com/job/compliance/job/gooddata-public-demos-oss-check/120/ : SUCCESS in 4s

Sonar scan result

No issue found. Congratulations!

More detail, see in https://sonarqube-gate.intgdc.com/dashboard?id=gooddata-public-demos-gate-PR62
To scan for vulnerabilities in dependencies and run unit tests (to get coverage report in sonar) please comment your PR with 'extended check sonar'.

Build succeeded (gate pipeline).

https://checklist.intgdc.com/job/compliance/job/gooddata-public-demos-merge-compliance-check/55/ : SUCCESS in 12s
https://checklist.intgdc.com/job/compliance/job/gooddata-public-demos-sonarqube-scan-zuul-gate/53/ : SUCCESS in 33s
https://checklist.intgdc.com/job/compliance/job/gooddata-public-demos-fossa-scanning-zuul/16/ : FAILURE in 3s (non-voting)


Failed to run with docker

I have a issue when i ran the docker:
this my compose.yaml :
services:
 matterbridge:
   image: ghcr.io/t0bst4r/home-assistant-matter-hub:latest
   restart: unless-stopped
   network_mode: host
   environment:
     - TZ=Europe/Paris
     - HAMH_HOME_ASSISTANT_URL="http://<IP_ADDRESS>:8100/"
     - HAMH_HOME_ASSITANT_ACCESS_TOKEN="eyJ...pdsEc"
     - HAMH_LOG_LEVEL="debug"
     - HAMH_WEB_PORT=8482
   volumes:
     - /home/test/matterbridge:/data

I have got the following error:
home-assistant-matter-hub start
start the application
Options:
  --help                         Show help                             [boolean]
  --log-level [string] [choices: "debug", "info", "warn", "error"] [default: "info"]
  --disable-log-colors                                [boolean] [default: false]
  --storage-location             Path to a directory where the application shoud store its data. Defaults to $HOME/.home-assistant-matter-hub                        [string]
  --web-port                     Port used by the web application [number] [default: 8482]
  --home-assistant-url           The HTTP-URL of your Home Assistant URL [string] [required]
  --home-assistant-access-token  A long-lived access token for your Home Assistant Instance                 [string] [required]
Unknown argument: homeAssitantAccessToken

ho right it's works :)


Fix mutable default arguments in http3_client example

Fix usage of mutable default arguments.
For an explanation of why mutable default arguments are a bad practice, see https://docs.python-guide.org/writing/gotchas/#mutable-default-arguments.

Hi! This PR makes sense, but could you please fix the linter errors and code coverage drop?

This PR makes sense, but it looks as though I need to fix a linter error unrelated to your changes.

@jlaine Sounds great. Feel free to merge main into this branch when you are done as needed, or ping me to do it.

Thanks for this @johnthagen , merged!


Container height jump to height of next component

Hi there,
I was struggling with this problem for a while until I noticed this at the bottom of your very helpful page :)
If the .*-height class (or transitionName.height) is not specified the change in container height will not be animated but instead jump to the height of the entering component instantaneously.
Still I'm not sure what to do here. It's happening on the carousel-swap, and I've tried going my css file and setting .carousel-swap { height:  }, but is that the way to go about it? I'm not sure either what the *-height class is? :)
Hope you can help!

Nikoflash

If you set up your replace container like this:
<ReactCSSTransitionReplace transitionName="carousel-swap" transitionEnterTimeout={500} transitionLeaveTimeout={500}>
  {someChangingKeyedContent}
</ReactCSSTransitionReplace>

Then you will need CSS transitions like this:
.carousel-swap-leave {
  transition: transform .3s ease-in-out;
  transform: translate(0, 0);
}
.carousel-swap-leave-active {
  transform: translate(-100%, 0);
}

.carousel-swap-enter {
  transition: transform .3s ease-in-out;
  transform: translate(100%, 0);
}
.carousel-swap-enter-active {
  transform: translate(0, 0);
}

.carousel-swap-height {
  transition: height .3s ease-in-out;
}

You create the animation the same as you would with react-css-transition-group with *-enter and *-leave styles, and then add an additional *-height style to animate the transition: height. If that transition isn't animated with CSS the height changes instantaneously.
I'm not sure if I understood your question correctly, so let me know if you're doing the above and still have the issue.

Hey thanks for the quick answer :)
I have gotten this working and sorry if my question was a little confusing, wasn't sure how to ask. So I think now that my actual problem is, that it doesn't animate like I expected it to do.
I was expecting the initial component to keep it's height, when it leaves and not animate to the size of the entering component. It looks like that is what is happening on your examples page with the carousel of two images.
In my case the leaving component is bigger than the entering component, and you can see the leaving component animate to the size of the entering, and I was just wondering if there is any way to avoid that? So the leaving component keeps its height as it animates out.

You could delay the height animation until the leave transition has completed:
.carousel-swap-height {
  transition: height .3s ease-in-out .3s;
}

The time at the end indicates the time to delay the animation, it's similar to what I did in this example.
Other than that I don't really know what to tell you. The reason I created this library was to enable the smooth transition in height while using some other transition to animate the replacement of the content. So in a way the "problem" you are having is, I think, the whole idea.
In the end, once you get when each of the CSS classes are added to the component, playing around with different combinations of durations and delays until you get what you're looking for is what it boils down to.

Haha, yeah I see your point :)
I am using your library for it's purpose, there was just this one transition where it didn't make sense with the height thing and then I had this "problem"
Maybe I should  just change the name of this issue to "I have a problem" and just leave it at that :)
thanks for the response though! 👍

It's all good. I hope you can manage to make it do what you want. If you figure something out and think others might benefit let me know and we can add it to the docs.


Filter out romaji reading from question

const items = [
  {
    meaning: ['tokyo', 'toukyou'],
    chars: '東京'
    reading: 'とうきょう'
  },
  {
    meaning: ['peace', 'heian'],
    chars: '平安'
    reading: 'へいあん'
  },
  {
    meaning: ['shibuya'],
    chars: '渋谷'
    reading: 'しぶや'
  }
]

function removeRedundancy({ meaning, reading }) {
  const filtered = meaning.filter(x => x !== toRomaji(reading));
  // guard against 1-1 mapping like 渋谷 -> ['shibuya'] becoming [] empty
  if (filtered.length) return filtered;
  return meaning;
}

items.map(removeRedundancy).forEach(({ meaning  }) => console.log(meaning));
// 'tokyo'
// 'peace'
// 'shibuya'

Done.


quota problem on min-io?

#76
I can create 50MiB of zarr file but I can't if it is 70MiB. This is the notebook which shows the problem.
https://github.com/pangeo-data/pangeo-eosc/blob/tinaok-patch-1/users/how-to/quota_pb_minio.ipynb
I would like to have a space
todaka-histogen, where I can write file, and other people can use my data.
How can I increase the quota?
How can I set bucket for doing this?
Thank you for your help.

Hi,
Re-running the notebook wrote the test.zarr into JupyterHub rather than MinIO. I am not sure how to update the code to properly write into MinIO.
On the other hand, I have also used this other example to test MinIO. Could you please give it a try and let me know how it goes?

Thank you @sebastian-luna-valero
It cannot handle large files. It works for me with files up to 50 MiB. Could you please try re-running the notebook with the following change?
size_in_MiB = 50

Change 50 to, for example, 500.
As seen in this image, I cannot change or know the quota of my space.

Do you know:
How to find out the quota for each bucket?
How to change the quota for each user's space?

Hi Tina,
Before troubleshooting further, these cells in your notebook:

Create a local folder in JupyterHub instead of MinIO:

Do you get the same?

Yes I get the same thing.
Sometime I can write sometime I can't. And I can't change the quota.

Hi Tina,
This is the problem, then. Your code is not writing to MinIO, and that's why you get the issue.
Could you please double check why your code is not writing to MinIO? In case it helps, this other example does write to MinIO.


Add parameters in header?

Fantastic library, thanks for sharing!
Was curious if it were possible to add values into the JOSE header, similar to https://github.com/panva/jose/blob/73da49330f940133ce257724a571fa29c0f0b98e/docs/interfaces/types.JWSHeaderParameters.md
I'm specifically looking to add a certificate identifier (x5t / x5u), which will make rotation that much easier.
Thank you!

Thanks for the fast response - makes sense and appreciated!


Make transaction history search form work via a GET request

so that google analytics can be used to monitor search parameters such, particularly the text-based search

Can I suggest to make the URL nicer?
instead of :
?received_at_0=16%2F11%2F2015&received_at_1=23%2F11%2F2015&search=blah&owner=
better:
?from=20151116&to=20151120&search=blah&owner=me

Can you confirm that django prevents DOM/script injections from the query string params?

RE injection: django escapes all form values unless explicitly marked as safe

RE GET params: we can change received_at_0 and received_at_1 but the number following that depends on what's input into the form field. it'd be weird to make people type dates specifically as 20151116… unless you set up a date picker and then pre-process the form on submission with JS

We can use tricks not to show the user confusing date formats on the form
while having them in the URL, but let's not bother just yet.
On 23 November 2015 at 13:41, ushkarev<EMAIL_ADDRESS>wrote:

RE GET params: we can change received_at_0 and received_at_1 but the
number following that depends on what's input into the form field. it'd be
weird to make people type dates specifically as 20151116… unless you set
up a date picker and then pre-process the form on submission with JS
—
Reply to this email directly or view it on GitHub
https://github.com/ministryofjustice/money-to-prisoners-cashbook/pull/107#issuecomment-158935660
.

In terms of the query string param formats, @ushkarev is right: we need a sensible format in the front-end. Ideally, something hyphen-separated (slashes get escaped in query strings).
I might also argue for from and to as the parameter names, rather than since and until, because they seem clearer? Or is there something I'm missing?

max and i were thinking that the date formatting is a slightly separate
issue which could go with the JS date picker task. the primary point of
this PR was to put the search text into the url for google analytics to
pick up.
On 24 November 2015 at 16:04, Steve Marshall<EMAIL_ADDRESS>wrote:

In terms of the query string param formats, @ushkarev
https://github.com/ushkarev is right: we need a sensible format in the
front-end. Ideally, something hyphen-separated (slashes get escaped in
query strings).
I might also argue for from and to as the parameter names, rather than
since and until, because they seem clearer? Or is there something I'm
missing?
—
Reply to this email directly or view it on GitHub
https://github.com/ministryofjustice/money-to-prisoners-cashbook/pull/107#issuecomment-159314709
.

as we were saying, from is a reserved keyword so it'd be overly hacky to
make it work.
On 24 November 2015 at 16:04, Steve Marshall<EMAIL_ADDRESS>wrote:

In terms of the query string param formats, @ushkarev
https://github.com/ushkarev is right: we need a sensible format in the
front-end. Ideally, something hyphen-separated (slashes get escaped in
query strings).
I might also argue for from and to as the parameter names, rather than
since and until, because they seem clearer? Or is there something I'm
missing?
—
Reply to this email directly or view it on GitHub
https://github.com/ministryofjustice/money-to-prisoners-cashbook/pull/107#issuecomment-159314709
.


dnsmasq introduction breaks applications running a local bind9 server and causes dns queries to fail.

The introduction of dnsmasq - breaks applications/devices which were earlier running a bind9 server in their container. There is a conflict of the ports.

Fixed in #57


QoL: Update Trade Items on C buttons and show the next Trade Item in Sequence in Inventory

Currently, if you press L or C-up to switch trade items while you have a trade item on a C button you have to reequip the trade item. This is technically fine, but I think it would be cleaner if the trade item updated on the C-button it's equipped to so the player doesn't have to reequip when wanting to switch trade items. This should apply to child, adult, and MM trade items. This also is technically preparation for the possibility of adding child trade items to D-pad right (if ageless items are not turned on).
Secondly, it would be nice if when you have multiple trade items on a slot it would show the next trade item in sequence at a smaller size next to it. MMR has this implemented for its trade item system and I think this would help communicate to new players that they haven't lost their trade item or have had it overwritten. The different colored icon isn't doing enough to communicate to new players, we still get questions about it every so often.

[x] Update C-button trade items when switching trade items
[ ] Add smaller sprites of the trade items, right next to the current trade item, to indicate the next trade item in sequence (and that you have more than one).

Example of task 2 (look at the Mountain Title Deed and Swamp Title Deed slot):

This is annoying to do with cross game icons. Do we really want that? As a compromise we could have a little "L"/"C-Up" icon when over a slot where this is possible, to avoid the !trade noob trap.

Done by https://github.com/OoTMM/OoTMM/compare/4c72d47a3f4c...04e2f79439f8


Migration to React 18

Added React 18
Added include-nodecg package.
Updated to Parcel 2
Updated react-query library
Fixed extension that was not compiling (its a temporary fix but I did not find any better way to do it).
Deleted Flux (consider the use of use-nodecg).
Updated the Readme.

I have forced the last push to sign the commit. Sorry.

Thank you so much man! let's see if we can also migrate parcel to parcel 2, im goin to start working on it

I have already updated to parcel 2 all settings.
I am adding generators for graphics and panels. I am doing it with inquirer


Redis backed AsyncPubSub

It might make sense to consider adding something analogous to the following:
https://www.apollographql.com/blog/backend/subscriptions/graphql-subscriptions-with-redis-pub-sub/
While AsyncPubSub is great for many use cases, in production where we might have multiple server instances, it may be necessary to use something like Redis instead of an in memory pubsub to handle subscriptions.  It would be cool if there could be an option to use Redis when configuring Pioneer and it would be even cooler if we could use the same API as AsyncPubSub.
So it would be a simple configuration change for the developer, and then we could switch between redis and the in memory AsyncPubSub without changing our EventStream code.
Do you think this is possible?  I know this might be a challenge.

It's very much possible. Pioneer can take any EventStream created from any AsyncSequence. Looking at this page of the Vapor docs, subscribing to a channel looked like
app.redis.subscribe(
  to: "channel_1", "channel_2",
  messageReceiver: { channel, message in
    switch channel {
    case "channel_1": // do something with the message
    default: break
    }
  },
  onUnsubscribe: { channel, subscriptionCount in
    print("unsubscribed from \(channel)")
    print("subscriptions remaining: \(subscriptionCount)")
  })

which is easy to translate to AsyncStream.
AsyncStream { con in
    app.redis.subscribe(
        to: ...,
        messageReceiver: { _, message in
            con.yield(message)
        },
        onUnsubscribe: { _, _ in
            con.finish()
        })

    ...
}

I think implementing something from that which have similar API to AsyncPubSub should be relatively straightforward. All you need to is to wrap that code into a method  named asyncStream and wrap the RediStack publish method into a method named publish.
The only problem is that I don't think it belong here. It make more sense as a separate package.
AsyncPubSub is provided in this library because I wanted to provide at least a basic in memory solution for Pub/Sub that can be used to start with immediately without extra setup.

I might try to make a separate package for this and maybe make AsyncPubSubBase protocol so that it will have similar API to AsyncPubSub.

I agree.  I think this approach makes a lot of sense.

I might try to make a separate package for this and maybe make AsyncPubSubBase protocol so that you can build something that will have similar API to AsyncPubSub, but I can't make any guarantees for that.

I have a working solutions for the public protocol from AsyncPubSub so that you can build a custom pubsub implementation with similar API to AsyncPubSub. More details on the changes is on #45
The changes are not final yet, I might change my mind on it to make it simpler. Would appreciate your thoughts on it.


Clarify jvm_test_suite_plugin documentation

I have a few things regarding the jvm_test_suite_plugin documentation.
First I wanted to ask this on stackoverflow, but then I thought that it might be worth to update the docs. So here we go:

It is not clear to me which framework is used per default for new test suites.
The documentation only mentions that JUnit4 is used in the built-in test suite for backwards compatibility reasons.
It seems that useJUnitJupiter() is the default for custom test suits, but it is not stated in the docs.

It is not clear to me what happens if useJUnitJupiter() is used. For example if I have already configured a specific junit jupiter version via spring-boot-dependencies and spring-boot-starter-test, which version will then be used? Or in other words, how can I let gradle use the libraries that are already on the test classpath.

Is it ok to mix both kind of configuration? Or does it have any disadvantages?
E.g. I liked the possibility to configure common parameters for all test tasks in one place.

testing {
    suites {

        val test by getting(JvmTestSuite::class) {
            useJUnitJupiter()
        }

        val integrationTest by registering(JvmTestSuite::class) {
            useJUnitJupiter()
            dependencies {
                implementation(project)
            }
        }

        val webMvcTest by registering(JvmTestSuite::class) {
            useJUnitJupiter()
            dependencies {
                implementation(project)
            }
        }
    }
}

tasks.withType<Test> {

    useJUnitPlatform()

    filter.isFailOnNoMatchingTests = false

    testLogging.exceptionFormat = TestExceptionFormat.FULL
    testLogging.showStandardStreams = false

    systemProperties["config.directory"] = if (isGitLab) ...
}

An example of how to use test fixtures for custom test suits would be nice.
I still have not figured out how to configure this. I expected something like this to work:

val integrationTest by registering(JvmTestSuite::class) {
    dependencies {
        implementation(project)
        // implementation(testFixtures(project)) this is not working as in `DependencyHandlerScope`.
    }
}

Regards, Daniel

@tresat could you please give an answer?

Thanks for your detailed explanation, it helped me a lot. Only this one is still not completely clear to me:

This is incorrect, you have to explicitly call this method to use Jupiter in additional test suites.

I tried this example from the docs with Gradle 7.3.3. and it seems that Jupiter is used as default for additional test suits, so I don't have to explicitly call this method. Regarding the build in test suite, everything behaves as stated in the docs. There I have to add useJUnitJupiter().
Did I understand correctly that after #19471 the default is Jupiter also for the default test suite?
Anyway, to be on the safe side, I will add useJUnitJupiter() to all test suites, as long as it is not the default everywhere.
Regarding the test fixtures, I am currently using the following workaround:
dependencies {
    add("webMvcTestImplementation", testFixtures(project))
    add("integrationTestImplementation", testFixtures(project))
}

That workaound should give you the same behavior.  Good luck.


Add Approve button for collect policies in the Settlement button

~On hold https://github.com/Expensify/Web-Expensify/pull/39328~
Details

This PR utilizes the NextStep logic from OldDot to show the "approve" or "pay" button based on the user role (either approver, reimburser, or both) and the reimbursement setup in the collect policy on NewDot (more details here). The approve button was moved to the SettlementButton to show it as another option in the dropdown selector.
This PR can only be tested by someone internal who can set up a control policy with an admin account to approve and reimburse the money requests from NewDot.
Fixed Issues

$ https://github.com/Expensify/App/issues/29145
PROPOSAL:
Tests

Pre-requisites (only in OldDot)

Create a collect policy
Execute the following JS snippet in the OldDot policy to enable workspace chats creation:

if (Url.getCurrentPageName()!="policy" ){ alert('Only run this snippet from a policy editor!'); } else { var p = Policy.getCurrent(); p.policy.isPolicyExpenseChatEnabled = "true"; p.save().done(function(){$.jGrowl('Workspace chat creation enabled!')}); }

Scenario 1: Approver as admin
Setup

Add a new collect policy admin (it can be the same person that created the policy).
Add the admin as Submit reports to: in Submit and Approve.
Add a new employee.
Add an OPEN bank account to the new collect policy admin by following this SO or by using clitools (only dev).
In OldDot go to Reimbursement, select Direct, and select the new collect policy admin as reimburser.
Set Manual reimbursement threshold to 0 (to test the Approval and then the Pay functionality).
Open NewDot in 2 different sessions; one for the approver and one for the member.

Tests (NewDot)

Sign-in with the member account in NewDot and create a manual request.
Verify there's no approve or pay button in the request.
Sign-in with the admin account, and verify the Approve option appears in the settlement button.
Approve the report and verify the message Cash - Approved appears in the report and request preview, the system message approved $x.xx appears in the expense report and the report header contains the approved text.
Pay the report and verify the report preview, request preview, and report header changed to paid, and also the system message is shown in the expense report.

https://github.com/Expensify/App/assets/6829422/354c779f-4207-48bb-b30e-76a9335fdfe0
Scenario 2: Approver as non-admin
Setup

Add a new member (Employee role) to the collect policy.
Add the new member as Submit reports to: in Submit and Approve.
Open NewDot in 3 different sessions; one for the reimburser, one for the approver and one for the member.

Tests (NewDot)

Sign in with the member account in NewDot and create a manual request.
Verify there's no approve or pay button in the request.
Sign in with the approver account, verify the Approve option appears in the settlement button.
Approve the report and verify the message Cash - Approved appears in the report and request preview, the system message approved $x.xx appears in the expense report and the report header contains the approved text.
Sign in with the reimburser account
Pay the report and verify the report preview, request preview, and report header changed to paid, and also the system message is shown in the expense report.

https://github.com/Expensify/App/assets/6829422/9d0711c9-8a13-4f9f-b382-d1302e1111fa

[ ] Verify that no errors appear in the JS console

Offline tests

QA Steps

[ ] Verify that no errors appear in the JS console

PR Author Checklist

[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 / Chrome
[x] iOS / native
[x] iOS / 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 approved by marketing by adding the Waiting for Copy label for a copy review on the original GH to get the correct copy.
[x] I verified proper file naming conventions were followed for any new files or renamed files. All non-platform specific files are named after what they export and are not named "index.js". All platform-specific files are named for the platform the code supports as outlined in the README.
[x] I verified the JSDocs style guidelines (in STYLE.md) were followed

[x] If a new code pattern is added I verified it was agreed to be used by multiple Expensify engineers
[x] I followed the guidelines as stated in the Review Guidelines
[x] I tested other components that can be impacted by my changes (i.e. if the PR modifies a shared library or component like Avatar, I verified the components using Avatar are working as expected)
[x] I verified all code is DRY (the PR doesn't include any logic written more than once, with the exception of tests)
[x] I verified any variables that can be defined as constants (ie. in CONST.js or at the top of the file that uses the constant) are defined as such
[x] I verified that if a function's arguments changed that all usages have also been updated correctly
[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] If we are not using the full Onyx data that we loaded, I've added the proper selector in order to ensure the component only re-renders when the data it is using changes

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

Screenshots/Videos

Web

https://github.com/Expensify/App/assets/6829422/085b4e52-d5c3-4d3b-8d10-81237f751dfa

Mobile Web - Chrome

Mobile Web - Safari

Desktop

iOS

Android

@marcochavezf I have some capacity issues at the moment, and I assume you would prefer a quick review. Would it be possible to reassign this one?

I am putting this PR on hold until the related bugs are fixed.

Hi @cubuspl42, the backend changes were deployed to staging and this PR can be tested now. Let me know if you have some capacity to review it, if so you can provide me a few emails so I can add you to the testing policy.

@marcochavezf
<EMAIL_ADDRESS><EMAIL_ADDRESS><EMAIL_ADDRESS><EMAIL_ADDRESS>

Perfect, thanks @cubuspl42 I added you to the collect policy Collect test (the workspace chats should appear in your accounts). The email<EMAIL_ADDRESS>is associated with another account<EMAIL_ADDRESS>as a secondary contact method, but I suppose that's not a problem if it's your account 🙂

I added<EMAIL_ADDRESS>as a non-admin approver:

So you could test scenario 2, until step 4, but that will be enough for the testing in this PR

Removing hold label since the backend changes were deployed to production

There's quite a delay between creating a request and being able to approve it on the approver side. First, I can see a "Pay" button, only if I re-enter this chat later the "Approve" option appears (and "Pay" disappears).
https://github.com/Expensify/App/assets/2590174/3060eb6e-8d3b-42f9-86e3-dffb052ef486

Oh interesting, seems the pay button is showing up in the workspace chat instead of the approve button, but the approve button is showing up correctly in the expense chat. I will check it out

I fixed the issue, there was a missing property for the settlement button in one place for small screens:

Thanks for catching that!

Am I doing something wrong now? I can't see the approve button at all on the approver side. Re-loading doesn't help.
https://github.com/Expensify/App/assets/2590174/5ec750e8-993e-4fab-aab5-ec37a9a4eec6

Hmm this is a weird error that's hard to reproduce with the local backend changes, seems there's another issue when the approver is non-admin. I updated your account<EMAIL_ADDRESS>as admin, can you test the scenario 1? The settlement button should now display 3 options (pay with expensify, pay elsewhere, and approver). Meanwhile, I will be checking out what's happening with the non-admin approver issue that you had.

@marcochavezf
Now I have both the "Pay" and "Approve" options, unlike in your video
Your video:

My case now:
https://github.com/Expensify/App/assets/2590174/ba3b8304-6341-433b-a02f-a6718988fb3d
Is this alright?

Hi @cubuspl42, I double-checked and yes this is expected because<EMAIL_ADDRESS>is now an admin, and you can either approve or pay the expense. We designed it in a way that optimizes direct payment (like regular IOUs or money requests in free workspaces), but includes the Approve option for cases where the users want an intermediate step. The video for scenario 1 shows the approve and pay buttons to the approver.

@marcochavezf Sorry, but something is not right here...
The second request gets "auto-approved", while it doesn't appear on the approver side at all.
https://github.com/Expensify/App/assets/2590174/0712896e-6abb-417a-be10-06860ab54353
I won't even mention the $1539 thing, as that number is out of the blue. 2 * 1234 PLN is not close to $1539.

@cubuspl42 I'm still investigating, for some reason we're grouping the submitted requests with the approved request. I let you know when it's fixed.

"Pay" / "Approve" (any action) button is missing on the preview until the report is entered:
This doesn't seem to be 100% reproducible...
https://github.com/Expensify/App/assets/2590174/7e1e3bee-1fa8-4a44-ac16-a2c3bd4215c2

The problem where the money request was grouped with the approved one has been fixed:
https://github.com/Expensify/App/assets/6829422/7fa5ba14-888c-4a42-8fa0-6ad0fe39990c

"Pay" / "Approve" (any action) button is missing on the preview until the report is entered:
This doesn't seem to be 100% reproducible...
approveButtonCollectPolicies-android-web-compressed.mp4

I'm still working on this issue, since we're not sending the NextStep data (which dictates if we'd show the Approve or Pay buttons) to the workspace chat. We're only sending them when the user opens the money request or via Pusher (real-time) when the member creates a money request.

Cool! Let me know when this is ready for the next round. I haven't performed the full cross-platform testing yet, as it would have to be repeated after non-trivial changes.

Hi @cubuspl42, I changed the approach a little bit here to fix the case where we were not showing the Pay/Approve buttons in the workspace chat. So it's ready for another review.
Additionally, I created another workspace to test scenario 2 (non-admin approver). I added your email accounts and<EMAIL_ADDRESS>is the non-admin approver:

Any issue please let me know, thanks!

Hi @cubuspl42, I noticed we shouldn't display the Pay button for scenario 1 (admin approver), because that account doesn't have a bank account shared with the reimburser. So let's ignore that scenario and just test scenario 2 (non-admin approver) please.

Thanks for the changes! I'll do another review & testing round this soon, sorry for the delay.

Hi @cubuspl42, this is ready for review again. And can be tested with both scenarios (admin approver and non-admin approver).
One bug that I was facing is when a member submits a report:
https://github.com/Expensify/App/assets/6829422/06991c6f-9434-439e-ac6a-af3dff4cfaba
This is a change introduced recently but it's not part of this PR cc @mountiny

Commenting here as I will doing the review

I am assuming I am not supposed to see any workspace as user, right?

I am assuming I am not supposed to see any workspace as user, right?

That's correct, as a non-admin member, you should only see the workspace in the LHN and not its configuration in the Workspace menu

https://github.com/Expensify/App/assets/58412969/4a510969-b0b8-46bd-bf8c-4978f819848e

I saw RBR when I try to change the category. Can you check this?

I saw RBR when I try to change the category. Can you check this?

Oh interesting, I think this is related to draft reports and it's out-of-scope of this PR, but thank you for point it out
Can you try to do the same after submitting the report? Just to ensure it's only for drafts?

https://github.com/Expensify/App/assets/58412969/0d0448fe-736c-4eb8-9da7-e77d46eba24e
And it is stuck on this unless I refresh

https://github.com/Expensify/App/assets/58412969/74f608cd-740c-46d9-ab9b-a496ed822a99
Looks like I am not getting RBR here

And it is stuck on this unless I refresh

Is this happening after you click on Submit the report from the member account?

And it is stuck on this unless I refresh

Is this happening after you click on Submit the report from the member account?

Yes

Looks like I am not getting RBR here

Ok, great then this is a bug for draft reports cc @mountiny, let's ignore it for this PR

https://github.com/Expensify/App/assets/58412969/a0d72db5-417b-4adb-95df-4cecce651a61
video before submit

And it is stuck on this unless I refresh

Is this happening after you click on Submit the report from the member account?

Yes

I will take a look at this one, meanwhile you can continue with the tests. These workspaces are behind a beta, so it's possible that a lot of weird things happen, but let's keep tracking them here

LHN subtext should also be shown approved instead of owed

See Avatar and there shouldn't be next steps shown here. I just paid from this screen to get this.

And text should be
Collected $23.00 elsewhere
instead of Collect workspace testing 1 paid $23.00 elsewhere

And text should be Collected $23.00 elsewhere
instead of Collect workspace testing 1 paid $23.00 elsewhere

I think this one is correct, no? Because for regular workspaces and IOUs we show the same pattern. In this case Collect workspace testing 1 is the name of the payer https://github.com/Expensify/App/blob/main/src/languages/en.ts#L577

And text should be Collected $23.00 elsewhere
instead of Collect workspace testing 1 paid $23.00 elsewhere

I think this one is correct, no? Because for regular workspaces and IOUs we show the same pattern. In this case Collect workspace testing 1 is the name of the payer https://github.com/Expensify/App/blob/main/src/languages/en.ts#L577

Oh sorry, I forgot the workspace name

For scenario 2, I can see all 3 buttons here.

Is this happening after you click on Submit the report from the member account?

the pusher events do not work correctly for draft reports, its know, for now just refresh as admin
The category selection is probably some validation error in BE
The paid message seems right to me

Out of nowhere, the submit failed

Now, the page looks like this

Should I see pay button without approve for scenario 1 and 2?

Only for the admin (who is not approver) in scenario 2

Out of nowhere, the submit failed

Oh interesting, I suppose this happened after submit or maybe after trying to submit a request that failed previously, but this should be also out-of-scope for this PR

Should I see pay button without approve for scenario 1 and 2?

Only for the admin (who is not approver) in scenario 2

Then for the first scenario, we should not see any pay buttons too.

See Avatar and there shouldn't be next steps shown here. I just paid from this screen to get this.

This is pretty inconsistent to reproduce.

Should I see pay button without approve for scenario 1 and 2?

Only for the admin (who is not approver) in scenario 2

Then for the first scenario, we should not see any pay buttons too.

Ah just for clarification,
For the first scenario, the approver (which is admin) should see Pay & Approve button
For the second scenario, the approver (non-admin) should only see the Approve button.

@marcochavezf Please ping here when you are done

This branch has conflicts that must be resolved

Once my comments are resolved
I will complete my checklist

Getting this error

Ok these are the bugs fixed:

[x] Show Finished! instead of Next Steps after a report is reimbursed
[x] nextStep.buttons prop console error
[x] Pay button is displayed in money request header when it shouldn't for admins who are not approvers

And these are these are the other bugs that are out-of-scope:

Category cannot be updated in draft reports link (draft reports implementation)
Report preview is not shown correctly for the approver after submitting a request link (draft reports implementation)
LHN subtext should also be shown approved instead of owed (asked internally if we should consider it a bug since technically the text is correct).

@shubham1206agra @cubuspl42 ready for review again 👍🏽

Not directly related to this ig. But putting here for full transparency

On approval, this strange error appeared on the requester side

Clicking on add your bank account does nothing.

See Avatar and there shouldn't be next steps shown here. I just paid from this screen to get this.

This problem is still there

Sometimes this error pops, looks like its coming from pusher (NAB)

We can disable the buttons to edit once the request is approved since these can't be edited from Employee side

Everything else looks fine.

https://github.com/Expensify/App/assets/58412969/6bab2ab9-bca1-412d-b77c-960ab29710a8
Can you check the copy text in this?

We can disable the buttons to edit once the request is approved since these can't be edited from Employee side

This is from an admin account, correct? If so it's fine because an admin can update the requests and we log the activity in the chat:

And thanks for finding the bugs! I will be working on them and let you know when are fixed 👍🏽

We can disable the buttons to edit once the request is approved since these can't be edited from Employee side

This is from an admin account, correct? If so it's fine because an admin can update the requests and we log the activity in the chat:

I am talking from employee side, not admin side

Bugs fixed:

[x] LHN subtext should also be shown approved instead of owed
[x] Subscript avatar should be shown for approved reports
[x] Push-to-page buttons should be disabled when the report is approved in the transaction chat
[x] Clipboard text is incorrect for approved reports

Reported errors out-of-scope (not introduced by the PR, but I will report in #expensify-bugs to corroborate if don't exist)

Prop transaction of type array supplied LHNOptionList, expected object https://github.com/Expensify/App/pull/28976#issuecomment-1837007964
The prop accountID is marked as required in ReportActionItemFragment, but its value is undefined https://github.com/Expensify/App/pull/28976#issuecomment-1837022207
Add bank account link not working (out-of-scope) https://github.com/Expensify/App/pull/28976#issuecomment-1837017821
TypeError: Failed to construct 'URL': Invalid URL (caused by the add your bank account link issue) https://github.com/Expensify/App/pull/28976#issuecomment-1837016932
Category cannot be updated in draft reports link (draft reports implementation)
Report preview is not shown correctly for the approver after submitting a request [link]

Other errors that I found and will address as follow-ups

Any admin can reimburse in the group policies (check if the admin has a VBBA account and that VBBA is set in the policy).
GBR is not cleared for approver after approving the report

@shubham1206agra @cubuspl42 PR ready for review again

Bump @shubham1206agra @cubuspl42

Push-to-page buttons should be disabled when the report is approved in the transaction chat
This is not happening when I am on request detail page. Only happens after going out and coming back

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 approved by marketing by adding the Waiting for Copy label for a copy review on the original GH to get the correct copy.
[ ] I verified proper file naming conventions were followed for any new files or renamed files. All non-platform specific files are named after what they export and are not named "index.js". All platform-specific files are named for the platform the code supports as outlined in the README.
[ ] I verified the JSDocs style guidelines (in STYLE.md) were followed

[ ] If a new code pattern is added I verified it was agreed to be used by multiple Expensify engineers
[ ] I 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(themeColors.componentBG)

[ ] If the PR modifies code that runs when editing or sending messages, I tested and verified there is no unexpected behavior for all supported markdown - URLs, single line code, code blocks, quotes, headings, bold, strikethrough, and italic.
[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 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.
[ ] I have checked off every checkbox in the PR reviewer checklist, including those that don't apply to this PR.

Screenshots/Videos

Testing scenario 1
https://github.com/Expensify/App/assets/58412969/cd467bf1-1aa7-4fb0-b3de-3b9fc109b7c3

Testing scenario 2
https://github.com/Expensify/App/assets/58412969/9d99abdb-0bc2-4ff9-a412-601f02fcc9e5

Can you check the transalations too? If it is in our scope or not.

Push-to-page buttons should be disabled when the report is approved in the transaction chat
This is not happening when I am on request detail page. Only happens after going out and coming back

This one is working fine for me, probably the pusher event wasn't coming for you?
https://github.com/Expensify/App/assets/6829422/9d5dec07-a711-459e-a2d4-5b44d11c2c3c

Can you check the transalations too? If it is in our scope or not.

Translations for report actions are out-of-scope for this PR, because some of the actions are generated in the server and don't support server translations atm.

Can you check the transalations too? If it is in our scope or not.

Translations for report actions are out-of-scope for this PR, because some of the actions are generated in the server and don't support server translations atm.

Let's put this in the list so we can report all these at once

Can you check the transalations too? If it is in our scope or not.

Translations for report actions are out-of-scope for this PR, because some of the actions are generated in the server and don't support server translations atm.

Let's put this in the list so we can report all these at once

Done 👍🏽

Push-to-page buttons should be disabled when the report is approved in the transaction chat
This is not happening when I am on request detail page. Only happens after going out and coming back

This one is working fine for me, probably the pusher event wasn't coming for you?
Screen.Recording.2023-12-06.at.11.24.55.a.m.mov

Now this is working fine

@shubham1206agra could you also include screenshots for the other platforms? Only from the approver user will be fine, I will include mine during the day

I can certainly add for mWeb, but for other platforms, you may need to start an adhoc build cause can't run 2 platforms on DEV at same time. Can you start an adhoc build if possible?

:test_tube::test_tube: Use the links below to test this adhoc build on Android, iOS, Desktop, and Web. Happy testing! :test_tube::test_tube:

Android :robot:
iOS :apple:

https://ad-hoc-expensify-cash.s3.amazonaws.com/android/28976/index.html
https://ad-hoc-expensify-cash.s3.amazonaws.com/ios/28976/index.html

Desktop :computer:
Web :spider_web:

❌ FAILED ❌
https://28976.pr-testing.expensify.com

The QR code can't be generated, because the Desktop build failed

Can you merge main again?

Oh one more thing. Have you confirmed the translations by someone, which are present in es.ts?

Yup, and are the same or similar to the translations that exist for Approve

Can you merge main again?

Done

Hi @shubham1206agra, would be possible for you to add the screenshots today?

:test_tube::test_tube: Use the links below to test this adhoc build on Android, iOS, Desktop, and Web. Happy testing! :test_tube::test_tube:

Android :robot:
iOS :apple:

https://ad-hoc-expensify-cash.s3.amazonaws.com/android/28976/index.html
https://ad-hoc-expensify-cash.s3.amazonaws.com/ios/28976/index.html

Desktop :computer:
Web :spider_web:

❌ FAILED ❌
https://28976.pr-testing.expensify.com

The QR code can't be generated, because the Desktop build failed

Yes

Cool, thank you @shubham1206agra! Now all yours @marcaaron

Thanks for the review! Updated and ready for review again

Oops like like prettier doesn't like the comments above the ternary operator

Maybe move the comment a line up

Maybe move the comment a line up

That was suggested here, but after applying the change the linter complained about it. Since this is just comment formatting, I will leave it as is.

: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/marcochavezf in version: 1.4.12-0 🚀

platform
result

🤖 android 🤖
success ✅

🖥 desktop 🖥
success ✅

🍎 iOS 🍎
cancelled 🔪

🕸 web 🕸
success ✅

🚀 Deployed to staging by https://github.com/marcochavezf in version: 1.4.12-0 🚀

platform
result

🤖 android 🤖
success ✅

🖥 desktop 🖥
success ✅

🍎 iOS 🍎
success ✅

🕸 web 🕸
success ✅

🚀 Deployed to production by https://github.com/Julesssss in version: 1.4.12-2 🚀

platform
result

🤖 android 🤖
success ✅

🖥 desktop 🖥
success ✅

🍎 iOS 🍎
success ✅

🕸 web 🕸
success ✅


Added some links to contributing guide, fixed setup (and changed it to a bash script)

Closes #222

@cauebs Considering the last comments I think it is totally up to you:

You can go ahead, git rm setup and add instructions to the CONTRIBUTING.md
Or just revert changes to setup and we handle that later

Besides that, thanks for the improvement in the documentation and to the port to the script that download the datasets ; )

Closed by inactivity


Implement redirection to game option as soon as the room capacity is reached.

this is not a server task


Added MultinomialNB_MinMaxScaler file.

Hello Neel, keep the data fetching part below initialization of feature and target. And add df.head() for original datafetch so that one can view original data before any process.

From the Multinomial remove blank code cell .


Specify marshal key in json tag

This affects generated 'api.go' files such that their keys, when json marshalled, are named as they were in the json shape definition.
For example, in ECS, the Cluster struct has the following diff:
type Cluster struct {
-       ClusterARN  *string `locationName:"clusterArn" type:"string" json:",omitempty"`
-       ClusterName *string `locationName:"clusterName" type:"string" json:",omitempty"`
-       Status      *string `locationName:"status" type:"string" json:",omitempty"`
+       ClusterARN  *string `locationName:"clusterArn" type:"string" json:"clusterArn,omitempty"`
+       ClusterName *string `locationName:"clusterName" type:"string" json:"clusterName,omitempty"`
+       Status      *string `locationName:"status" type:"string" json:"status,omitempty"`

       	metadataCluster `json:"-", xml:"-"`
 }

I also checked what it sends over the wire and verified it changed as expected for the subset of shapes I tried. This change matches what boto sends over the wire as well (again for that subset).
It's possible there is a better way to accomplish this or there's further ramifications I don't realize.

This is right for now. The builtin JSON marshaller will eventually be replaced with custom code to more consistently follow the locationName and other traits that we have setup for other protocols. Until then, updating the json tag seems fine.

Thanks!

I'd argue that there's some value in keeping it usable with the default json marshaller/unmarshaller as well unless there's some significant reason to move to the custom one you mention.
Sticking with 'json' helps the structs interop with other software more cleanly.
My case is certainly a very unusual one, but I'm using an alternate transport and it's convenient to be able to write these structs as json over a transport with the go std library as opposed to a custom aws marshaller.

The AWS wire protocol (with all of its potential quirks) should not be a concern for the default JSON marshaller. In other words, the fact that clusterName is lowercased on the wire in AWS requests should not be important to a user trying to marshal a Cluster struct to JSON outside of AWS. They probably want "ClusterName", as it appears in the Go struct name. If anything, tying those JSON tags directly to the struct makes things worse for interop, because the SDK is now enforcing that same AWS wire protocol whenever a user wants to serialize the object as a structure to any other source. There wire protocol should be an implementation detail that isn't visible outside of the SDK.
For a concrete example, consider the REST-JSON protocol. In this protocol, structure members that map to headers must be omitted from a serialized JSON body (because they are part of headers, not the body), and as such, the "Go way" to do it is to add the json:"-" tag to this field. This works out great, allowing us to omit this field from body serialization (after we have read/written it from/into a header). It might look like this:
type Email struct {
    Sender string `json:"-"` // because this comes from the "email" header in req/responses
    // ...
}

The only problem with this is that now a user cannot take that structure and serialize it in their JSON logs or to disk, or any other source, because this important exported field that is part of the API has now been marked as "ignored" from all JSON serialization by the SDK:
b, err := json.Marshal(resp)
fmt.Println(string(b)) // Prints "{}" even when "Email" is set.

Playground link
This is problematic for any kind of data use of these structs outside the context of the SDK, so in order not to break the above use case, we're trying to stay away from tying the wire protocol to Go's marshalers.

@euank just a heads up here: the change in this pull request has effectively been reverted due to the discussion above. If you're relying on these tags being present in some custom logic, note that the logic will no longer be valid.
We should have a discussion about your use case so we can figure out what if anything we can do to support the customizations you need. Or maybe there are alternative routes that would allow us to avoid changes in the generator codebase. Shoot me an email and let's chat.


PineconeInternalServerError: An internal server error occurred while calling the describe_index_stats endpoint

Is this a new bug?

[X] I believe this is a new bug
[X] I have searched the existing issues, and I could not find an existing issue for this bug

Current Behavior
Our service is calling describeIndexStats() function which is getting failed after some duration. I searched and found there is one mention but not any reply on this thread as well. https://community.pinecone.io/t/an-internal-server-error-occured-while-calling-the-endpoint/4499
Please help

Expected Behavior
This should not occur when getting index stats.
Steps To Reproduce

In the GCP cloud run with : Ubuntu 20.04
With node:21-slim
with Pinecone TS client pinecone-database/pinecone 1.1.2

Relevant log output
Error getting index stats PineconeInternalServerError: An internal server error occured while calling the https://issues-003-large-429ed45.svc.apw5-4e34-81fa.pinecone.io/describe_index_stats endpoint. To see overall service health and learn whether this seems like a large-scale problem or one specific to your request, please go to https://status.pinecone.io/ to view our status page. If you believe the error reflects a problem with this client, please file a bug report in the github issue tracker at https://github.com/pinecone-io/pinecone-ts-client

Environment
- **GCP Cloud Run Ubuntu 20.04**:
- **node:21-slim**:
- **@pinecone-database/pinecone": "^1.1.2**:

Additional Context
No response

@coderabbit-amank Are you still hitting this issue?