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

Add navigatum link to description #175

Merged

Conversation

Pydes-boop
Copy link
Contributor

This is a proposed solution that resolves #165.

  • Adds a regex that matches exactly the pattern that all roomcodes at tum share (like: (5612.03.017), (5612.EG.017), (5612.EG.010B)) which are used for navigaTUM URLs.
  • improves readability of location tests

I did some local tests and manually checked some additional ICS files generated with the proposed changes additionally and it all looked fine, but i would love it if someone else could build this proposal locally and also do a quick check.

Im was undecided if the roomcode should be removed from the description whenever we add a navigatum link but for now i decided to keep it.

Im quite new to Go so i hope i did everything fairly alright :)

Copy link
Member

@CommanderStorm CommanderStorm left a comment

Choose a reason for hiding this comment

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

Change looks good (=working, bugfree) to me.
I have left a few nits inline

I double-checked which IDs are not matched by this regex and have not found any rooms (just buildings/poi, but those won't have a calendar entry anyway)

- root
- lmu
- 0050
- 1003
- stammgelaende
- nordgelaende
- 0101
- 0102
- 0103
- 0104
- 0105
- 0106
- 0108
- 0109
- suedgelaende
- 0201
- 0202
- 0203
- 0204
- 0205
- 0206
- 2920
- suedostgelaende
- 0305
- suedwestgelaende
- 0401
- 0402
- 0403
- 2903
- 2910
- 2911
- 2912
- 2913
- zentralgelaende
- 0501
- 0502
- 0503
- 0504
- 0505
- 0506
- 0507
- 0508
- 0509
- 0510
- 0511
- 0512
- mri
- 1501
- 1502
- 1503
- 1504
- 1507
- 1508
- 1509
- 1514
- 1516
- mri-psychosomatik
- 1518
- 1527
- 2522
- 1523
- 1524
- 1531
- 1532
- 1533
- 1535
- 1536
- 1537
- 1538
- 1539
- 1540
- 1541
- 1542
- 1543
- 1544
- 1545
- 1546
- 1547
- 1548
- 1549
- 1551
- 1552
- 1555
- 1555a
- 1555b
- 1555c
- 1556
- 1557
- 1558
- 1559
- 1560
- 1561
- 1711
- 1712
- 1713
- 1716
- 1717
- 1718
- 1719
- 1720
- 1724
- 1725
- 1740
- biederstein
- 1601
- 1602
- 1603
- 1607
- 1608
- 1650
- 1652
- 1708
- heilbronn
- 1901
- 1902
- 1910
- 1915
- 1998
- 1999
- kapuzinerhoelzl
- 2103
- 2104
- 2105
- 2106
- 2107
- 2108
- 2109
- 2201
- 2202
- olympiapark
- 2309
- 2310
- 2311
- 2312
- 2313
- 2315
- 2317
- 2318
- 2319
- 2320
- 2321
- 2330
- 2331
- 2332
- 2333
- 2334
- 2350
- 2351
- 2352
- uptown-muenchen
- 2940
- 2941
- 2353
- 2354
- holzforschung
- 2401
- 2402
- 2410
- pasing
- 2601
- 2602
- 2604
- 2605
- 2607
- 2608
- grosshadern
- 2701
- 2801
- 2803
- 2804
- 2805
- 2806
- 2807
- 2808
- innenstadt-sonstige
- 2905
- 2906
- 2907
- 2908
- 2909
- 2926
- 2928
- 2944
- cs
- 2927
- 2929
- 2930
- 3501
- 3502
- 3503
- 3504
- 3505
- 3515
- 3537
- 3035
- 3100
- obernach
- 3101
- 3102
- 3103
- 3104
- 3105
- 3106
- 3107
- 3108
- 3109
- 3110
- 3111
- 3112
- 3113
- 3114
- 3115
- 3116
- 3117
- 3118
- 3119
- 3120
- 3121
- 3122
- 3123
- 3124
- 3201
- residenz
- 3401
- 3402
- starnberg
- 3901
- 3902
- 3904
- wzw
- 4001
- wzw-berg
- wzw-berg-alte-akademie
- 4101
- 4102
- 4103
- 4105
- 4106
- 4107
- 4108
- wzw-berg-weihenstephaner-steig
- 4109
- 4110
- 4111
- 4113
- 4114
- 4115
- 4116
- 4117
- wzw-berg-hohenbachernstr
- 4119
- 4120
- wzw-ziel
- 4124
- 4126
- 4128
- 4130
- aquatische-systembiologie
- 4129
- 4131
- 4132
- 4153
- 4155
- 4156
- 4176
- veitshof
- 4180
- 4181
- 4182
- 4183
- 4184
- 4185
- 4186
- 4187
- 4188
- 4189
- 4190
- 4191
- veitshof-wasserwerk
- 4192
- wzw-mitte
- 4202
- 4205
- 4210
- 4211
- 4212
- 4213
- 4214
- 4215
- 4216
- 4217
- 4218
- 4219
- 4220
- 4221
- 4222
- 4223
- 4224
- 4225
- 4226
- 4238
- 4239
- 4254
- 4259
- 4264
- 4267
- 4275
- 4277
- 4278
- 4279
- 4281
- 4299
- wzw-nord
- 4304
- wzw-lbs
- 4307
- 4308
- 4317
- 4318
- 4309
- 4310
- 4311
- 4314
- wzw-lagerhalle
- 4315
- 4316
- 4319
- 4320
- wzw-gm
- 4321
- 4322
- 4323
- 4324
- 4325
- wzw-lfl
- 4353
- 4355
- 4361
- 4362
- 4368
- 4379
- 4387
- staudensichtungsgarten
- wzw-extern
- iffeldorf
- 4401
- 4402
- 4403
- 4404
- 4405
- viehhausen
- 4521
- 4522
- 4523
- 4524
- thalhausen
- 4601
- 4602
- 4603
- 4604
- 4605
- 4606
- 4607
- 4608
- 4609
- 4610
- 4611
- 4612
- 4613
- 4614
- 4615
- 4616
- 4620
- 4800
- gruenschwaige
- 4801
- 4802
- 4803
- 4804
- 4805
- 4806
- 4807
- 4808
- 4809
- 4810
- 4811
- 4812
- 4813
- 4814
- 4815
- roggenstein
- 4901
- 4902
- 4903
- 4907
- 4908
- 4909
- 4910
- 4914
- 4915
- 4916
- 4920
- duernast
- 4127
- 4230
- 4231
- 4232
- 4233
- 4234
- 4235
- 4236
- 4237
- 4501
- 4502
- 4503
- 4505
- 4506
- 4508
- 4509
- 4510
- 4512
- 4513
- garching
- physik
- physik-main
- 5101
- 5107
- 5103
- 5104
- 5108
- 5109
- 5111
- 5112
- 5115
- 5116
- mll
- 5120
- 5121
- 5122
- 5123
- 5124
- 5125
- 5126
- 5130
- 5131
- 5140
- 5142
- 5143
- 5160
- mpi
- ipp
- mpa
- mpb
- mpe
- mpp
- mpq
- origins-cluster
- frm-2
- 5201
- 5202
- 5203
- 5204
- 5207
- 5208
- 5209
- 5210
- 5211
- 5212
- 5214
- 5215
- 5218
- 5219
- 5220
- 5221
- 5222
- 5224
- 5225
- 5226
- 5227
- 5228
- 5229
- 5231
- 5232
- 5233
- 5234
- 5235
- 5236
- 5237
- 5250
- 5251
- 5252
- 5253
- 5256
- 5257
- 5268
- 5272
- 5301
- 5302
- 5304
- 5305
- chemie
- 5401
- 5402
- 5403
- 5404
- 5405
- 5406
- 5407
- chemie-nebengebaeude
- 5408
- 5409
- 5410
- 5413
- 5414
- 5415
- 5433
- mw
- 5501
- 5502
- 5503
- 5504
- 5505
- 5506
- 5507
- 5508
- 5509
- 5510
- 5513
- 5514
- 5515
- 5517
- 5518
- 5519
- 5530
- 5531
- 5532
- mi
- 5601
- 5602
- 5603
- 5604
- 5605
- 5606
- 5607
- 5608
- 5609
- 5610
- 5611
- 5612
- 5613
- 5622
- 5701
- 5801
- 5901
- 5932
- galileo
- 8120
- 8121
- 8122
- 8123
- 8124
- garching-interims
- 5416
- 5539
- 5620
- garching-sued-west
- garching-gebaeudemanagement
- 6101
- 6102
- 6103
- 6104
- 6106
- 6107
- feuerwehrwache-garching
- 6202
- 6203
- 6204
- 6206
- garching-extra
- 7894
- 7895
- 7896
- 7910
- garching-hochbrueck
- 8101
- 8102
- 8104
- 8111
- 9001
- 9101
- 9201
- taufkirchen-ottobrunn
- 9376
- 9377
- 9379
- 9378
- 9380
- 9390
- 5515.U1.1598
- 4503.EG.03-43
- 0305.04.S28A
- 3035.EG.8004
- 5506.03.6980
- 4234.EG.53
- 0101.EG.N0126A
- 0101.EG.N0161
- 0101.EG.N0178
- parabelrutsche
- validierungsautomat-1
- validierungsautomat-2
- validierungsautomat-3
- validierungsautomat-4
- validierungsautomat-5
- validierungsautomat-6
- validierungsautomat-7
- validierungsautomat-8
- validierungsautomat-9
- validierungsautomat-10
- validierungsautomat-11
- validierungsautomat-12
- validierungsautomat-14
- validierungsautomat-16
- validierungsautomat-21
- validierungsautomat-sot
- validierungsautomat-straubing

internal/app_test.go Outdated Show resolved Hide resolved
internal/app.go Outdated Show resolved Hide resolved
internal/app.go Outdated Show resolved Hide resolved
@CommanderStorm CommanderStorm merged commit ae300d3 into TUM-Dev:master Apr 9, 2024
1 check passed
@CommanderStorm
Copy link
Member

Thanks for the PR, change will be online in a few minutes. 🎉
Caching in calendar apps is quite severe => expect the change there in a few days..weeks

@Pydes-boop
Copy link
Contributor Author

Thank you so much! Glad it was such a straightforward change 😄

@Pydes-boop Pydes-boop deleted the feature/navigatum-link-description branch April 14, 2024 22:54
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.

add navigatum link
2 participants