[Copilot No. Series] Add Json functionality for working with JSON data #716

DmitryKatson · 2024-03-11T07:02:44Z

Add new Json module to provide tools for working with JSON data, including reading, writing, and parsing JSON. Introduces codeunit for initializing JSON array and object, retrieving element counts, fetching objects by index, and getting values for specified record fields. Implements methods to interact with JSON data using .NET types.

This is partial implementation of theBase Application codeunit 5459 "Json management"

Summary

This PR is required for the Number Series Copilot implementation as we agreed here

Work Item(s)

Fixes #715

Fixes AB#506714

Add new Json module to provide tools for working with JSON data, including reading, writing, and parsing JSON. Introduces codeunit for initializing JSON array and object, retrieving element counts, fetching objects by index, and getting values for specified record fields. Implements methods to interact with JSON data using .NET types. This is partly ported from Base Application codeunit 5459 "Json management"

DmitryKatson · 2024-03-11T07:26:31Z

@microsoft-github-policy-service agree

JesperSchulz

Tests missing before code review can continue.

src/System Application/App/Json/src/Json.Codeunit.al

Added app.json file defining a JSON module and its dependencies, as well as a new codeunit "Json Test" containing tests for JSON functionality, specifically around JSON collection manipulation and value extraction. This enhances the system's testing capabilities around JSON processing.

Add individual functions to retrieve various data types from JSON objects, like strings, boolean, integer, decimals etc.

src/System Application/App/Json/src/JsonImpl.Codeunit.al

JesperSchulz · 2024-03-13T13:00:23Z

I'm generally happy with this module! Clean API, well tested!

If you get rid of the last two codecop issues which prevent compilation, I'll get another set of eyes on this and in it goes.

…bles order declaration The GetObjectFromCollectionByIndex method signature has been modified to accept the index parameter before the JSON object in text format parameter. This change aims to improve consistency and adherence to best practices.

DmitryKatson · 2024-03-13T14:04:54Z

I'm generally happy with this module! Clean API, well tested!

If you get rid of the last two codecop issues which prevent compilation, I'll get another set of eyes on this and in it goes.

Pushed commit with fixes. Please take a look

src/System Application/App/Json/src/Json.Codeunit.al

…pulate JSON arrays Added functions to modify, add, or remove properties, as well as manipulate JSON arrays. Extracted JSON array and object retrieval methods to improve code readability and maintainability. These changes enhance the flexibility and reusability of JSON handling.

DmitryKatson · 2024-03-14T15:20:03Z

New commit 354ecd2 is pushed with the fixes on the provided comments and also new apis, together with new tests

JesperSchulz · 2024-03-15T08:17:14Z

@duichwer, are you happy with this PR now? If you are, I'll do some manual testing and will do a detailed review, and then we're good to go here, I'd say.

duichwer · 2024-03-15T08:54:48Z

In general I'm Okay with this PR for now.
Will it be possible to have an overload for the procedure GetObject in the future that returns the native AL JsonObject in the future?

I'm never sure which overloads are possible in AL.
Same applies for the GetCollection procedure.

If this isn't possible then I'd suggest to rename those procedures to something like GetObjectAsText

Besides that I'm totally fine with this PR.

JesperSchulz · 2024-03-15T10:38:53Z

Two more build issues:
Error: AA0137 Variable 'IsHandled' is unused in 'AddJPropertyToJObject'.
Error: AA0137 Variable 'JArray' is unused in 'AddJObjectToCollection'.

We're getting there :)

JesperSchulz · 2024-03-15T10:42:54Z

In general I'm Okay with this PR for now. Will it be possible to have an overload for the procedure GetObject in the future that returns the native AL JsonObject in the future?

I'm never sure which overloads are possible in AL. Same applies for the GetCollection procedure.

If this isn't possible then I'd suggest to rename those procedures to something like GetObjectAsText

Besides that I'm totally fine with this PR.

You cannot overload on the return type alone. The parameters must be different. Why not go with the "AsText" suggestion? It makes it very clear in which shape you'll get the object / collection. I'd vote for that.

Refactor JSON codeunit methods to return JSON array and object directly as well as in text format, enhancing flexibility and usability. Update related test cases accordingly.

DmitryKatson · 2024-03-15T10:58:54Z

In general I'm Okay with this PR for now. Will it be possible to have an overload for the procedure GetObject in the future that returns the native AL JsonObject in the future?
I'm never sure which overloads are possible in AL. Same applies for the GetCollection procedure.
If this isn't possible then I'd suggest to rename those procedures to something like GetObjectAsText
Besides that I'm totally fine with this PR.

You cannot overload on the return type alone. The parameters must be different. Why not go with the "AsText" suggestion? It makes it very clear in which shape you'll get the object / collection. I'd vote for that.

It's done.
previous GetCollection and GetObject renamed to GetCollectionAsText and GetObjectAsText accordinly. New GetCollection and GetObject added with JsonArray and JsonObject return values.

Tests updated. Please check

grobyns · 2024-03-19T11:07:37Z

We've got these enabled by default:

"al.codeAnalyzers":  [
                         "${AppSourceCop}",
                         "${CodeCop}"
                     ],

We also have PTECop and UICop enabled but I think the thing you need is to use the same ruleset as we do:
the GitHub AL:Go! setup looks like this:

"enableCodeCop":  true,
"enableAppSourceCop":  true,
"enablePerTenantExtensionCop":  true,
"enableUICop":  true,
"enableCodeAnalyzersOnTestApps":  true,
"rulesetFile":  "..\\..\\..\\src\\rulesets\\ruleset.json",

JesperSchulz · 2024-03-19T11:12:49Z

We've got these enabled by default:
"al.codeAnalyzers":  [
                         "${AppSourceCop}",
                         "${CodeCop}"
                     ],
We also have PTECop and UICop enabled but I think the thing you need is to use the same ruleset as we do: the GitHub AL:Go! setup looks like this:
"enableCodeCop":  true,
"enableAppSourceCop":  true,
"enablePerTenantExtensionCop":  true,
"enableUICop":  true,
"enableCodeAnalyzersOnTestApps":  true,
"rulesetFile":  "..\\..\\..\\src\\rulesets\\ruleset.json",

My apologies. When I was looking into the settings file, I was working on the BaseApp. Addendum: The ruleset used here is base.ruleset.json, which I don't know if we're actually shipping (will find out).

For the System App and Business Foundation, we've indeed got all 4 cops enabled with the internal.module.ruleset.json when compiling modules individually, and the ruleset.json when compiling the entire layer into an app.

This I indeed should document :-)

Simplify and restructure JSON parsing methods to enhance readability and provide greater flexibility for handling JSON data. These changes also aim to improve maintainability and extensibility of the codebase. No related issues.

src/System Application/App/Json/src/JsonImpl.Codeunit.al

JesperSchulz

Looks fine to me now. Approved!

src/System Application/App/Json/app.json

Adding .NET dependency.

src/System Application/App/Json/src/Json.Codeunit.al

src/System Application/App/Json/src/JsonImpl.Codeunit.al

Co-authored-by: Darrick <[email protected]>

Consolidate variable declaration to enhance code readability and maintainability. This change eliminates redundancy and streamlines the code structure.

src/System Application/App/Json/src/Json.Codeunit.al

src/System Application/App/Json/src/JsonImpl.Codeunit.al

src/System Application/Test/Json/src/JsonTest.Codeunit.al

Apply namespace changes Co-authored-by: Darrick <[email protected]>

m3koenig · 2024-03-28T10:11:16Z

I like this one! Are there usage examples avaibalbe anywhere (asides from the Tests :D)?
Would be helpful.

DmitryKatson · 2024-03-29T02:34:48Z

I like this one! Are there usage examples avaibalbe anywhere (asides from the Tests :D)? Would be helpful.

You can find some additional examples here or just search for :Json: Codeunit Json;. Later, when this be ready, it will be merged to master

DmitryKatson requested a review from a team as a code owner March 11, 2024 07:02

github-actions bot added AL: System Application From Fork Pull request is coming from a fork labels Mar 11, 2024

This was referenced Mar 11, 2024

[BC Idea]: Json Module #715

Closed

[BC Idea]: Number Series Copilot #659

Closed

JesperSchulz linked an issue Mar 11, 2024 that may be closed by this pull request

[BC Idea]: Json Module #715

Closed

1 task

JesperSchulz requested changes Mar 11, 2024

View reviewed changes

src/System Application/App/Json/src/Json.Codeunit.al Show resolved Hide resolved

grobyns reviewed Mar 11, 2024

View reviewed changes

src/System Application/App/Json/src/Json.Codeunit.al Show resolved Hide resolved

DmitryKatson added 2 commits March 11, 2024 20:12

Add specific value retrieval functions from JSON object

88ea0dd

Add individual functions to retrieve various data types from JSON objects, like strings, boolean, integer, decimals etc.

JesperSchulz reviewed Mar 13, 2024

View reviewed changes

src/System Application/App/Json/src/JsonImpl.Codeunit.al Outdated Show resolved Hide resolved

JesperSchulz added the Linked Issue is linked to a Azure Boards work item label Mar 13, 2024

JesperSchulz assigned JesperSchulz and DmitryKatson Mar 13, 2024

duichwer reviewed Mar 14, 2024

View reviewed changes

src/System Application/App/Json/src/Json.Codeunit.al Outdated Show resolved Hide resolved

duichwer reviewed Mar 14, 2024

View reviewed changes

src/System Application/App/Json/src/Json.Codeunit.al Outdated Show resolved Hide resolved

duichwer reviewed Mar 14, 2024

View reviewed changes

src/System Application/App/Json/src/Json.Codeunit.al Show resolved Hide resolved

Refactor JSON codeunit methods for improved functionality

b536471

Refactor JSON codeunit methods to return JSON array and object directly as well as in text format, enhancing flexibility and usability. Update related test cases accordingly.

JesperSchulz mentioned this pull request Mar 18, 2024

[Copilot No. Series] Number series copilot #764

Merged

JesperSchulz changed the title ~~Add Json functionality for working with JSON data~~ [Copilot No. Series] Add Json functionality for working with JSON data Mar 20, 2024

JesperSchulz assigned darjoo Mar 20, 2024

JesperSchulz reviewed Mar 22, 2024

View reviewed changes

src/System Application/App/Json/src/JsonImpl.Codeunit.al Show resolved Hide resolved

JesperSchulz previously approved these changes Mar 22, 2024

View reviewed changes

JesperSchulz reviewed Mar 22, 2024

View reviewed changes

src/System Application/App/Json/app.json Show resolved Hide resolved

Update src/System Application/App/Json/app.json

ce02948

Adding .NET dependency.

JesperSchulz dismissed their stale review via ce02948 March 22, 2024 11:34

darjoo reviewed Mar 22, 2024

View reviewed changes

DmitryKatson and others added 2 commits March 22, 2024 19:24

Apply suggestions from code review

af3f8eb

Co-authored-by: Darrick <[email protected]>

Refactor code to consolidate variable declaration in JsonTest

d684ae3

Consolidate variable declaration to enhance code readability and maintainability. This change eliminates redundancy and streamlines the code structure.

darjoo reviewed Mar 22, 2024

View reviewed changes

src/System Application/App/Json/src/Json.Codeunit.al Outdated Show resolved Hide resolved

src/System Application/App/Json/src/JsonImpl.Codeunit.al Outdated Show resolved Hide resolved

src/System Application/Test/Json/src/JsonTest.Codeunit.al Outdated Show resolved Hide resolved

Apply suggestions from code review

d56f591

Apply namespace changes Co-authored-by: Darrick <[email protected]>

JesperSchulz previously approved these changes Mar 22, 2024

View reviewed changes

darjoo previously approved these changes Mar 22, 2024

View reviewed changes

JesperSchulz enabled auto-merge (squash) March 22, 2024 13:31

github-actions bot added this to the Version 25.0 milestone Mar 22, 2024

Update JsonTest.Codeunit.al

b9d9d74

JesperSchulz dismissed stale reviews from darjoo and themself via b9d9d74 March 22, 2024 13:50

JesperSchulz approved these changes Mar 22, 2024

View reviewed changes

darjoo approved these changes Mar 22, 2024

View reviewed changes

JesperSchulz merged commit 858a482 into microsoft:main Mar 22, 2024
23 checks passed

JesperSchulz added the Integration GitHub request for Integration area label May 16, 2024

DmitryKatson deleted the Json-Mgt-Impl branch September 14, 2024 05:56

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

[Copilot No. Series] Add Json functionality for working with JSON data #716

[Copilot No. Series] Add Json functionality for working with JSON data #716

DmitryKatson commented Mar 11, 2024 •

edited by github-actions bot

Loading

DmitryKatson commented Mar 11, 2024

JesperSchulz left a comment

JesperSchulz commented Mar 13, 2024

DmitryKatson commented Mar 13, 2024

DmitryKatson commented Mar 14, 2024

JesperSchulz commented Mar 15, 2024

duichwer commented Mar 15, 2024

JesperSchulz commented Mar 15, 2024

JesperSchulz commented Mar 15, 2024

DmitryKatson commented Mar 15, 2024

grobyns commented Mar 19, 2024 •

edited

Loading

JesperSchulz commented Mar 19, 2024 •

edited

Loading

JesperSchulz left a comment

m3koenig commented Mar 28, 2024

DmitryKatson commented Mar 29, 2024

[Copilot No. Series] Add Json functionality for working with JSON data #716

[Copilot No. Series] Add Json functionality for working with JSON data #716

Conversation

DmitryKatson commented Mar 11, 2024 • edited by github-actions bot Loading

Summary

Work Item(s)

DmitryKatson commented Mar 11, 2024

JesperSchulz left a comment

Choose a reason for hiding this comment

JesperSchulz commented Mar 13, 2024

DmitryKatson commented Mar 13, 2024

DmitryKatson commented Mar 14, 2024

JesperSchulz commented Mar 15, 2024

duichwer commented Mar 15, 2024

JesperSchulz commented Mar 15, 2024

JesperSchulz commented Mar 15, 2024

DmitryKatson commented Mar 15, 2024

grobyns commented Mar 19, 2024 • edited Loading

JesperSchulz commented Mar 19, 2024 • edited Loading

JesperSchulz left a comment

Choose a reason for hiding this comment

m3koenig commented Mar 28, 2024

DmitryKatson commented Mar 29, 2024

DmitryKatson commented Mar 11, 2024 •

edited by github-actions bot

Loading

grobyns commented Mar 19, 2024 •

edited

Loading

JesperSchulz commented Mar 19, 2024 •

edited

Loading