Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Adapt the Network UI to the HTTP/JSON API #1116

Merged
merged 17 commits into from
Apr 9, 2024
Merged

Conversation

teclator
Copy link
Contributor

@teclator teclator commented Mar 25, 2024

Problem

Recently, we have been adding an HTTP/JSON layer on top of the network configuration. Using this new API we should be able to:

Display the active connections in the NetworkSection and the ConnectionsTable.
Allow editing the configuration of those connections.

Solution

The UI has been adapted using the HTTP/JSON API and additing or updating a connections using it is now allowed. Although it was out of the scope, adding a Wireless connection is also allowed but in that case we are not listening to the DBUS events and not reacting properly, therefore, a refresh would be needed in order to see the added connection.

@coveralls
Copy link

coveralls commented Mar 25, 2024

Coverage Status

coverage: 70.467% (-0.4%) from 70.823%
when pulling 21e6941 on adapt_network_UI
into ccea4e9 on architecture_2024.

@imobachgs imobachgs changed the title Adapt the Network UI using the http API Adapt the Network UI to the HTTP/JSON API Apr 3, 2024
Copy link
Member

@imobachgs imobachgs left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Just a partial review; I will continue in a while.

web/src/client/network/index.js Outdated Show resolved Hide resolved
rust/agama-lib/src/network/types.rs Outdated Show resolved Hide resolved
rust/agama-server/src/network/model.rs Show resolved Hide resolved
rust/agama-server/src/network/nm/client.rs Outdated Show resolved Hide resolved
Copy link
Member

@imobachgs imobachgs left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Already reviewed the Rust code. Let's go with the JavaScript part.

rust/agama-server/src/network/nm/client.rs Show resolved Hide resolved
rust/agama-server/src/network/web.rs Outdated Show resolved Hide resolved
rust/agama-server/src/network/web.rs Outdated Show resolved Hide resolved
rust/agama-server/src/network/web.rs Outdated Show resolved Hide resolved
.unwrap();

if let Some(mut current_conn) = rx.await.unwrap() {
current_conn.set_up();
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Would not be better to put this logic as part of the action? IMHO, as much as possible, the handler should be responsible of parsing/validating the request/response and call the right method (or dispatch the right action).

However, if we have followed that approach elsewhere, we can keep it as it is and reevaluate in the short term (when we drop the D-Bus code and clean-up the actions).

rust/agama-server/src/network/web.rs Outdated Show resolved Hide resolved
rust/agama-server/src/network/web.rs Outdated Show resolved Hide resolved
web/src/client/network.test.js Outdated Show resolved Hide resolved
Copy link
Member

@imobachgs imobachgs left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Finished with reviewing the JavaScript code too.

rust/agama-server/src/network/web.rs Outdated Show resolved Hide resolved
web/src/client/http.js Outdated Show resolved Hide resolved
@teclator teclator marked this pull request as ready for review April 8, 2024 14:01
@teclator teclator requested a review from imobachgs April 8, 2024 16:31
@teclator teclator merged commit 7df6f83 into architecture_2024 Apr 9, 2024
4 checks passed
@teclator teclator deleted the adapt_network_UI branch April 9, 2024 08:50
@teclator teclator mentioned this pull request Apr 9, 2024
teclator added a commit that referenced this pull request Apr 9, 2024
## Problem

After merged #1116 the network model test is broken

## Solution

Fixed test
imobachgs added a commit that referenced this pull request May 6, 2024
After a few months of work, it is time to merge the `architecture_2024`
branch into `master`. It is still a work-in-progress, but all the
efforts should be go now against that branch.

## Pull requests

* #1061
* #1064
* #1073
* #1074
* #1080
* #1089
* #1091
* #1092
* #1094
* #1095
* #1099
* #1100
* #1102
* #1103
* #1112
* #1114
* #1116
* #1117
* #1119
* #1120
* #1123
* #1126
* #1129
* #1130
* #1131
* #1132
* #1133
* #1134
* #1136
* #1139
* #1140
* #1143
* #1146

## Other commits

* 8efa41f
* 9e2dec0
@imobachgs imobachgs mentioned this pull request May 17, 2024
imobachgs added a commit that referenced this pull request May 17, 2024
Prepare for releasing Agama 8. It includes the following pull requests:

* #884
* #886
* #914
* #918
* #956
* #957
* #958
* #959
* #960
* #961
* #962
* #963
* #964
* #965
* #966
* #969
* #970
* #976
* #977
* #978
* #979
* #980
* #981
* #983
* #984
* #985
* #986
* #988
* #991
* #992
* #995
* #996
* #997
* #999
* #1003
* #1004
* #1006
* #1007
* #1008
* #1009
* #1010
* #1011
* #1012
* #1014
* #1015
* #1016
* #1017
* #1020
* #1022
* #1023
* #1024
* #1025
* #1027
* #1028
* #1029
* #1030
* #1031
* #1032
* #1033
* #1034
* #1035
* #1036
* #1038
* #1039
* #1041
* #1042
* #1043
* #1045
* #1046
* #1047
* #1048
* #1052
* #1054
* #1056
* #1057
* #1060
* #1061
* #1062
* #1063
* #1064
* #1066
* #1067
* #1068
* #1069
* #1071
* #1072
* #1073
* #1074
* #1075
* #1079
* #1080
* #1081
* #1082
* #1085
* #1086
* #1087
* #1088
* #1089
* #1090
* #1091
* #1092
* #1093
* #1094
* #1095
* #1096
* #1097
* #1098
* #1099
* #1100
* #1102
* #1103
* #1104
* #1105
* #1106
* #1109
* #1110
* #1111
* #1112
* #1114
* #1116
* #1117
* #1118
* #1119
* #1120
* #1121
* #1122
* #1123
* #1125
* #1126
* #1127
* #1128
* #1129
* #1130
* #1131
* #1132
* #1133
* #1134
* #1135
* #1136
* #1138
* #1139
* #1140
* #1141
* #1142
* #1143
* #1144
* #1145
* #1146
* #1147
* #1148
* #1149
* #1151
* #1152
* #1153
* #1154
* #1155
* #1156
* #1157
* #1158
* #1160
* #1161
* #1162
* #1163
* #1164
* #1165
* #1166
* #1167
* #1168
* #1169
* #1170
* #1171
* #1172
* #1173
* #1174
* #1175
* #1177
* #1178
* #1180
* #1181
* #1182
* #1183
* #1184
* #1185
* #1187
* #1188
* #1189
* #1190
* #1191
* #1192
* #1193
* #1194
* #1195
* #1196
* #1198
* #1199
* #1200
* #1201
* #1203
* #1204
* #1205
* #1206
* #1207
* #1208
* #1209
* #1210
* #1211
* #1212
* #1213
* #1214
* #1215
* #1216
* #1217
* #1219
* #1220
* #1221
* #1222
* #1223
* #1224
* #1225
* #1226
* #1227
* #1229
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

3 participants